[PATCH] D92634: [Analyzer] Diagnose signed integer overflow

[Daniel Marjamäki via Phabricator via cfe-commits]

danielmarjamaki created this revision.
danielmarjamaki added reviewers: NoQ, xazax.hun.
Herald added subscribers: steakhal, ASDenysPetrov, martong, Charusso, dkrupp, 
donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, rnkovacs, szepet, 
baloghadamsoftware.
danielmarjamaki requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Diagnose signed integer overflows. The core.UndefResultChecker will warn.

This is a bit unfinished.. I wonder if you like the approach. It only checks 
addition right now. and not underflow.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D92634

Files:
  clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
  clang/test/Analysis/integer-overflow.c

Index: clang/test/Analysis/integer-overflow.c
===
--- /dev/null
+++ clang/test/Analysis/integer-overflow.c
@@ -0,0 +1,7 @@
+// RUN: %clang_analyze_cc1 -analyze -analyzer-checker core.UndefinedBinaryOperatorResult -verify %s
+
+void f(int x) {
+if (x > 0x7f00 && x + 32 < 0x7fff){}
+if (x > 0x7ff0 && x + 32 < 0x7fff){} // expected-warning {{The result of the '+' expression is undefined}}
+}
+
Index: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
===
--- clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
+++ clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
@@ -42,6 +42,8 @@
   SVal evalBinOpLN(ProgramStateRef state, BinaryOperator::Opcode op,
Loc lhs, NonLoc rhs, QualType resultTy) override;
 
+  bool isGreater(ProgramStateRef State, const SymExpr *LHS, int64_t RHS);
+
   /// getKnownValue - evaluates a given SVal. If the SVal has only one possible
   ///  (integer) value, that value is returned. Otherwise, returns NULL.
   const llvm::APSInt *getKnownValue(ProgramStateRef state, SVal V) override;
@@ -51,7 +53,8 @@
   SVal simplifySVal(ProgramStateRef State, SVal V) override;
 
   SVal MakeSymIntVal(const SymExpr *LHS, BinaryOperator::Opcode op,
- const llvm::APSInt , QualType resultTy);
+ const llvm::APSInt , QualType resultTy,
+ ProgramStateRef State);
 };
 } // end anonymous namespace
 
@@ -210,14 +213,29 @@
   }
 }
 
+bool SimpleSValBuilder::isGreater(ProgramStateRef State, const SymExpr *LHS,
+  int64_t RHS) {
+  ProgramStateManager  = State->getStateManager();
+  SValBuilder  = Mgr.getSValBuilder();
+  SVal Eval =
+  Bldr.evalBinOp(State, BO_GT, nonloc::SymbolVal(LHS),
+ makeIntVal(RHS, LHS->getType()), Bldr.getConditionType());
+  if (Eval.isUnknownOrUndef())
+return false;
+  ProgramStateRef StTrue, StFalse;
+  std::tie(StTrue, StFalse) = State->assume(Eval.castAs());
+  return (StTrue && !StFalse);
+}
+
 //===--===//
 // Transfer function for binary operators.
 //===--===//
 
 SVal SimpleSValBuilder::MakeSymIntVal(const SymExpr *LHS,
-BinaryOperator::Opcode op,
-const llvm::APSInt ,
-QualType resultTy) {
+  BinaryOperator::Opcode op,
+  const llvm::APSInt ,
+  QualType resultTy,
+  ProgramStateRef State) {
   bool isIdempotent = false;
 
   // Check for a few special cases with known reductions first.
@@ -281,6 +299,15 @@
   if (isIdempotent)
   return evalCastFromNonLoc(nonloc::SymbolVal(LHS), resultTy);
 
+  // Is there a signed integer overflow
+  if (LHS->getType()->isSignedIntegerType() && op == BO_Add) {
+int64_t ResultTyMaxVal =
+(1ULL << (getContext().getTypeSize(resultTy) - 1)) - 1;
+if (RHS > 0 && RHS < ResultTyMaxVal &&
+isGreater(State, LHS, ResultTyMaxVal - RHS.getExtValue()))
+  return UndefinedVal();
+  }
+
   // If we reach this point, the expression cannot be simplified.
   // Make a SymbolVal for the entire expression, after converting the RHS.
   const llvm::APSInt *ConvertedRHS = 
@@ -748,7 +775,7 @@
   }
 
   // Otherwise, make a SymIntExpr out of the expression.
-  return MakeSymIntVal(symIntExpr, op, *RHSValue, resultTy);
+  return MakeSymIntVal(symIntExpr, op, *RHSValue, resultTy, state);
 }
   }
 
@@ -764,7 +791,7 @@
 
   // Is the RHS a constant?
   if (const llvm::APSInt *RHSValue = getKnownValue(state, rhs))
-return MakeSymIntVal(Sym, op, *RHSValue, resultTy);
+return MakeSymIntVal(Sym, op, *RHSValue, resultTy, state);
 
   if (Optional V = tryRearrange(state, op, lhs, rhs, resultTy))
 return *V;
@@ -938,7 +965,7 @@
   // build an expression for 

[PATCH] D92633: Add -f[no-]direct-access-external-data to deprecate -mpie-copy-relocations

[Sriraman Tallam via Phabricator via cfe-commits]

tmsriram added a comment.

You said : "The name -mpie-copy-relocations is misleading [1] and does not 
capture the idea that this option can actually apply to all of -fno-pic,-fpie, 
..."

Could you please clarify why this option needs to apply to -fno-pic?  Here is 
what I tried with trunk clang:

extern int var;
int get() { return var; }

$ clang -S foo.c -o -

movlvar, %eax
popq%rbp
...

With -fno-pic,  this will never need to use -mpie-copy-relocations at all, so 
this case is not relevant right?  Did I miss anything?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92633

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

[PATCH] D88394: [Driver][M68k] (Patch 8/8) Add driver support for M68k

[Min-Yih Hsu via Phabricator via cfe-commits]

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



Comment at: clang/include/clang/Driver/Options.td:3125
+foreach i = {0-4} in
+  def m680#i#0 : Flag<["-"], "m680"#i#"0">, Group;
 

myhsu wrote:
> bruno wrote:
> > rengolin wrote:
> > > Same question as @RKSimon had below: Shouldn't this cover all models the 
> > > back-end recognises?
> > Unless you are planning to add 100 or more target variations I'd prefer to 
> > see these explicitly defined instead of a `foreach`. If I'm grepping for a 
> > specific CPU variation in the code base it's nice to get that information 
> > easily. 
> @rengolin  I think the backend currently doesn't support M68060 either
Update: I've just put the sub-target (skeleton) for M68060. So now you can 
specific M68060 :-)



Comment at: clang/lib/Driver/ToolChains/Arch/M68k.cpp:44
+
+llvm::Regex CPUPattern("m?680([01234]{1})0");
+llvm::SmallVector Matches;

RKSimon wrote:
> Why no 68060 ?
@RKSimon  Just added 060's support :-)
Currently I don't see any major ISA difference between 060 and 040 (correct me 
if I'm wrong). So I just put a sub-target skeleton and clang support for 060, 
and fill in the details in the future


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

https://reviews.llvm.org/D88394

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

[PATCH] D88394: [Driver][M68k] (Patch 8/8) Add driver support for M68k

[Min-Yih Hsu via Phabricator via cfe-commits]

myhsu updated this revision to Diff 309462.
myhsu added a comment.

- Add support for M68060 sub-target


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

https://reviews.llvm.org/D88394

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/CMakeLists.txt
  clang/lib/Driver/ToolChains/Arch/M68k.cpp
  clang/lib/Driver/ToolChains/Arch/M68k.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/lib/Driver/ToolChains/Linux.cpp
  clang/test/Driver/m68k-features.cpp
  clang/test/Driver/m68k-sub-archs.cpp

Index: clang/test/Driver/m68k-sub-archs.cpp
===
--- /dev/null
+++ clang/test/Driver/m68k-sub-archs.cpp
@@ -0,0 +1,35 @@
+// RUN: %clang -### -target m68k-unknown-linux -mcpu=68000 %s 2>&1 | FileCheck --check-prefix=CHECK-MX00 %s
+// RUN: %clang -### -target m68k-unknown-linux -mcpu=m68000 %s 2>&1 | FileCheck --check-prefix=CHECK-MX00 %s
+// RUN: %clang -### -target m68k-unknown-linux -mcpu=M68000 %s 2>&1 | FileCheck --check-prefix=CHECK-MX00 %s
+// RUN: %clang -### -target m68k-unknown-linux -m68000 %s 2>&1 | FileCheck --check-prefix=CHECK-MX00 %s
+// CHECK-MX00: "-target-cpu" "M68000"
+
+// RUN: %clang -### -target m68k-unknown-linux -mcpu=68010 %s 2>&1 | FileCheck --check-prefix=CHECK-MX10 %s
+// RUN: %clang -### -target m68k-unknown-linux -mcpu=m68010 %s 2>&1 | FileCheck --check-prefix=CHECK-MX10 %s
+// RUN: %clang -### -target m68k-unknown-linux -mcpu=M68010 %s 2>&1 | FileCheck --check-prefix=CHECK-MX10 %s
+// RUN: %clang -### -target m68k-unknown-linux -m68010 %s 2>&1 | FileCheck --check-prefix=CHECK-MX10 %s
+// CHECK-MX10: "-target-cpu" "M68010"
+
+// RUN: %clang -### -target m68k-unknown-linux -mcpu=68020 %s 2>&1 | FileCheck --check-prefix=CHECK-MX20 %s
+// RUN: %clang -### -target m68k-unknown-linux -mcpu=m68020 %s 2>&1 | FileCheck --check-prefix=CHECK-MX20 %s
+// RUN: %clang -### -target m68k-unknown-linux -mcpu=M68020 %s 2>&1 | FileCheck --check-prefix=CHECK-MX20 %s
+// RUN: %clang -### -target m68k-unknown-linux -m68020 %s 2>&1 | FileCheck --check-prefix=CHECK-MX20 %s
+// CHECK-MX20: "-target-cpu" "M68020"
+
+// RUN: %clang -### -target m68k-unknown-linux -mcpu=68030 %s 2>&1 | FileCheck --check-prefix=CHECK-MX30 %s
+// RUN: %clang -### -target m68k-unknown-linux -mcpu=m68030 %s 2>&1 | FileCheck --check-prefix=CHECK-MX30 %s
+// RUN: %clang -### -target m68k-unknown-linux -mcpu=M68030 %s 2>&1 | FileCheck --check-prefix=CHECK-MX30 %s
+// RUN: %clang -### -target m68k-unknown-linux -m68030 %s 2>&1 | FileCheck --check-prefix=CHECK-MX30 %s
+// CHECK-MX30: "-target-cpu" "M68030"
+
+// RUN: %clang -### -target m68k-unknown-linux -mcpu=68040 %s 2>&1 | FileCheck --check-prefix=CHECK-MX40 %s
+// RUN: %clang -### -target m68k-unknown-linux -mcpu=m68040 %s 2>&1 | FileCheck --check-prefix=CHECK-MX40 %s
+// RUN: %clang -### -target m68k-unknown-linux -mcpu=M68040 %s 2>&1 | FileCheck --check-prefix=CHECK-MX40 %s
+// RUN: %clang -### -target m68k-unknown-linux -m68040 %s 2>&1 | FileCheck --check-prefix=CHECK-MX40 %s
+// CHECK-MX40: "-target-cpu" "M68040"
+
+// RUN: %clang -### -target m68k-unknown-linux -mcpu=68060 %s 2>&1 | FileCheck --check-prefix=CHECK-MX60 %s
+// RUN: %clang -### -target m68k-unknown-linux -mcpu=m68060 %s 2>&1 | FileCheck --check-prefix=CHECK-MX60 %s
+// RUN: %clang -### -target m68k-unknown-linux -mcpu=M68060 %s 2>&1 | FileCheck --check-prefix=CHECK-MX60 %s
+// RUN: %clang -### -target m68k-unknown-linux -m68060 %s 2>&1 | FileCheck --check-prefix=CHECK-MX60 %s
+// CHECK-MX60: "-target-cpu" "M68060"
Index: clang/test/Driver/m68k-features.cpp
===
--- /dev/null
+++ clang/test/Driver/m68k-features.cpp
@@ -0,0 +1,45 @@
+// Check macro definitions
+// RUN: %clang -target m68k-unknown-linux -m68000 -dM -E %s | FileCheck --check-prefix=CHECK-MX %s
+// CHECK-MX: #define __mc68000 1
+// CHECK-MX: #define __mc68000__ 1
+// CHECK-MX: #define mc68000 1
+
+// RUN: %clang -target m68k-unknown-linux -m68010 -dM -E %s | FileCheck --check-prefix=CHECK-MX10 %s
+// CHECK-MX10: #define __mc68000 1
+// CHECK-MX10: #define __mc68000__ 1
+// CHECK-MX10: #define __mc68010 1
+// CHECK-MX10: #define __mc68010__ 1
+// CHECK-MX10: #define mc68000 1
+// CHECK-MX10: #define mc68010 1
+
+// RUN: %clang -target m68k-unknown-linux -m68020 -dM -E %s | FileCheck --check-prefix=CHECK-MX20 %s
+// CHECK-MX20: #define __mc68000 1
+// CHECK-MX20: #define __mc68000__ 1
+// CHECK-MX20: #define __mc68020 1
+// CHECK-MX20: #define __mc68020__ 1
+// CHECK-MX20: #define mc68000 1
+// CHECK-MX20: #define mc68020 1
+
+// RUN: %clang -target m68k-unknown-linux -m68030 -dM -E %s | FileCheck --check-prefix=CHECK-MX30 %s
+// CHECK-MX30: #define __mc68000 1
+// CHECK-MX30: #define __mc68000__ 1
+// CHECK-MX30: #define __mc68030 1
+// CHECK-MX30: #define __mc68030__ 1
+// CHECK-MX30: #define mc68000 1
+// CHECK-MX30: 

[PATCH] D88393: [cfe][M68k] (Patch 7/8) Basic Clang support

[Min-Yih Hsu via Phabricator via cfe-commits]

myhsu updated this revision to Diff 309461.
myhsu added a comment.

- Add support for M68060 sub-target


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

https://reviews.llvm.org/D88393

Files:
  clang/include/clang/Basic/Attr.td
  clang/lib/Basic/CMakeLists.txt
  clang/lib/Basic/Targets.cpp
  clang/lib/Basic/Targets/M68k.cpp
  clang/lib/Basic/Targets/M68k.h
  clang/lib/CodeGen/TargetInfo.cpp
  clang/lib/Sema/SemaDeclAttr.cpp

Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -6306,6 +6306,39 @@
   D->addAttr(::new (S.Context) MipsInterruptAttr(S.Context, AL, Kind));
 }
 
+static void handleM68kInterruptAttr(Sema , Decl *D, const ParsedAttr ) {
+  if (!checkAttributeNumArgs(S, AL, 1))
+return;
+
+  if (!AL.isArgExpr(0)) {
+S.Diag(AL.getLoc(), diag::err_attribute_argument_type)
+<< AL << AANT_ArgumentIntegerConstant;
+return;
+  }
+
+  // FIXME: Check for decl - it should be void ()(void).
+
+  Expr *NumParamsExpr = static_cast(AL.getArgAsExpr(0));
+  auto MaybeNumParams = NumParamsExpr->getIntegerConstantExpr(S.Context);
+  if (!MaybeNumParams) {
+S.Diag(AL.getLoc(), diag::err_attribute_argument_type)
+<< AL << AANT_ArgumentIntegerConstant
+<< NumParamsExpr->getSourceRange();
+return;
+  }
+
+  unsigned Num = MaybeNumParams->getLimitedValue(255);
+  if ((Num & 1) || Num > 30) {
+S.Diag(AL.getLoc(), diag::err_attribute_argument_out_of_bounds)
+<< AL << (int)MaybeNumParams->getSExtValue()
+<< NumParamsExpr->getSourceRange();
+return;
+  }
+
+  D->addAttr(::new (S.Context) M68kInterruptAttr(S.Context, AL, Num));
+  D->addAttr(UsedAttr::CreateImplicit(S.Context));
+}
+
 static void handleAnyX86InterruptAttr(Sema , Decl *D, const ParsedAttr ) {
   // Semantic checks for a function with the 'interrupt' attribute.
   // a) Must be a function.
@@ -6578,6 +6611,9 @@
   case llvm::Triple::mips:
 handleMipsInterruptAttr(S, D, AL);
 break;
+  case llvm::Triple::m68k:
+handleM68kInterruptAttr(S, D, AL);
+break;
   case llvm::Triple::x86:
   case llvm::Triple::x86_64:
 handleAnyX86InterruptAttr(S, D, AL);
Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -8066,6 +8066,43 @@
   return false;
 }
 
+//===--===//
+// M68k ABI Implementation
+//===--===//
+
+namespace {
+
+class M68kTargetCodeGenInfo : public TargetCodeGenInfo {
+public:
+  M68kTargetCodeGenInfo(CodeGenTypes )
+  : TargetCodeGenInfo(std::make_unique(CGT)) {}
+  void setTargetAttributes(const Decl *D, llvm::GlobalValue *GV,
+   CodeGen::CodeGenModule ) const override;
+};
+
+} // namespace
+
+void M68kTargetCodeGenInfo::setTargetAttributes(
+const Decl *D, llvm::GlobalValue *GV, CodeGen::CodeGenModule ) const {
+  if (const auto *FD = dyn_cast_or_null(D)) {
+if (const auto *attr = FD->getAttr()) {
+  // Handle 'interrupt' attribute:
+  llvm::Function *F = cast(GV);
+
+  // Step 1: Set ISR calling convention.
+  F->setCallingConv(llvm::CallingConv::M68k_INTR);
+
+  // Step 2: Add attributes goodness.
+  F->addFnAttr(llvm::Attribute::NoInline);
+
+  // Step 3: Emit ISR vector alias.
+  unsigned Num = attr->getNumber() / 2;
+  llvm::GlobalAlias::create(llvm::Function::ExternalLinkage,
+"__isr_" + Twine(Num), F);
+}
+  }
+}
+
 //===--===//
 // AVR ABI Implementation.
 //===--===//
@@ -10877,6 +10914,8 @@
 
   case llvm::Triple::le32:
 return SetCGInfo(new PNaClTargetCodeGenInfo(Types));
+  case llvm::Triple::m68k:
+return SetCGInfo(new M68kTargetCodeGenInfo(Types));
   case llvm::Triple::mips:
   case llvm::Triple::mipsel:
 if (Triple.getOS() == llvm::Triple::NaCl)
Index: clang/lib/Basic/Targets/M68k.h
===
--- /dev/null
+++ clang/lib/Basic/Targets/M68k.h
@@ -0,0 +1,57 @@
+//===--- M68k.h - Declare M68k target feature support ---*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This file declares M68k TargetInfo objects.
+//
+//===--===//
+
+#ifndef M680X0_H_LTNCIPAD
+#define M680X0_H_LTNCIPAD
+
+#include 

[PATCH] D92633: Add -f[no-]direct-access-external-data to deprecate -mpie-copy-relocations

[Fangrui Song via Phabricator via cfe-commits]

MaskRay created this revision.
MaskRay added reviewers: hjl.tools, rnk, tmsriram.
Herald added subscribers: dexonsmith, dang, pengfei, s.egerton, simoncook.
MaskRay requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

GCC r218397 "x86-64: Optimize access to globals in PIE with copy reloc" made
-fpie code emit R_X86_64_PC32 to reference external data symbols by default.
Clang adopted -mpie-copy-relocations D19996  
as a flexible alternative.

The name -mpie-copy-relocations is misleading [1] and does not capture the idea
that this option can actually apply to all of -fno-pic,-fpie,-fpie [2], so this
patch introduces -f[no-]direct-access-external-data and makes
-mpie-copy-relocations their aliases for compatibility.

[1]
For

  extern int var;
  int get() { return var; }

if var is defined in another translation unit in the link unit, there is no copy
relocation.

[2]
While -fno-pic code generally uses an absolute relocation to reference a data
symbol, GOT indirection can still be used.

-fpic can avoid GOT indirection as well, the user should be responsible for
defining var in another translation unit, otherwise the linker will error in a
-shared link (copy relocations are not available in shared objects).

Neither the -fno-pic nor the -fpic behavior is implemented in this patch, 
though.

GCC feature request https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98112 (the
description is much more complex than the Clang description because GCC/GNU ld
x86-64 have some STV_PROTECTED issues which originated from the default
HAVE_LD_PIE_COPYRELOC thing).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D92633

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/dso-local-executable.c
  clang/test/Driver/fdirect-access-external-data.c

Index: clang/test/Driver/fdirect-access-external-data.c
===
--- /dev/null
+++ clang/test/Driver/fdirect-access-external-data.c
@@ -0,0 +1,18 @@
+/// -fno-pic code defaults to -fdirect-access-external-data.
+// RUN: %clang -### -c -target x86_64 %s 2>&1 | FileCheck %s --check-prefix=DEFAULT
+// RUN: %clang -### -c -target x86_64 %s -fdirect-access-external-data 2>&1 | FileCheck %s --check-prefix=DEFAULT
+// RUN: %clang -### -c -target x86_64 %s -fdirect-access-external-data -fno-direct-access-external-data 2>&1 | FileCheck %s --check-prefix=INDIRECT
+
+/// -fpie/-fpic code defaults to -fdirect-access-external-data.
+// RUN: %clang -### -c -target x86_64 %s -fpie 2>&1 | FileCheck %s --check-prefix=DEFAULT
+// RUN: %clang -### -c -target x86_64 %s -fpie -fno-direct-access-external-data -fdirect-access-external-data 2>&1 | FileCheck %s --check-prefix=DIRECT
+// RUN: %clang -### -c -target aarch64 %s -fpic 2>&1 | FileCheck %s --check-prefix=DEFAULT
+// RUN: %clang -### -c -target aarch64 %s -fpic -fdirect-access-external-data 2>&1 | FileCheck %s --check-prefix=DIRECT
+
+/// -m[no-]pie-copy-relocations are aliases for compatibility.
+// RUN: %clang -### -c -target riscv64 %s -mno-pie-copy-relocations 2>&1 | FileCheck %s --check-prefix=INDIRECT
+// RUN: %clang -### -c -target riscv64 %s -fpic -mpie-copy-relocations 2>&1 | FileCheck %s --check-prefix=DIRECT
+
+// DEFAULT-NOT: direct-access-external-data"
+// DIRECT:  "-fdirect-access-external-data"
+// INDIRECT:"-fno-direct-access-external-data"
Index: clang/test/CodeGen/dso-local-executable.c
===
--- clang/test/CodeGen/dso-local-executable.c
+++ clang/test/CodeGen/dso-local-executable.c
@@ -42,7 +42,7 @@
 // PIE-DAG: define dso_local i32* @zed()
 // PIE-DAG: declare void @import_func()
 
-// RUN: %clang_cc1 -triple x86_64 -emit-llvm -pic-level 1 -pic-is-pie -mpie-copy-relocations %s -o - | FileCheck --check-prefix=PIE-DIRECT %s
+// RUN: %clang_cc1 -triple x86_64 -emit-llvm -pic-level 1 -pic-is-pie -fdirect-access-external-data %s -o - | FileCheck --check-prefix=PIE-DIRECT %s
 // PIE-DIRECT:  @baz = dso_local global i32 42
 // PIE-DIRECT-NEXT: @import_var = external dso_local global i32
 // PIE-DIRECT-NEXT: @weak_bar = extern_weak global i32
@@ -64,7 +64,7 @@
 // NOPLT-DAG: define dso_local i32* @zed()
 // NOPLT-DAG: declare void @import_func()
 
-// RUN: %clang_cc1 -triple x86_64 -emit-llvm -fno-plt -pic-level 1 -pic-is-pie -mpie-copy-relocations %s -o - | FileCheck --check-prefix=PIE-DIRECT-NOPLT %s
+// RUN: %clang_cc1 -triple x86_64 -emit-llvm -fno-plt -pic-level 1 -pic-is-pie -fdirect-access-external-data %s -o - | FileCheck --check-prefix=PIE-DIRECT-NOPLT %s
 // PIE-DIRECT-NOPLT:  @baz = dso_local global i32 42
 // PIE-DIRECT-NOPLT-NEXT: @import_var = external dso_local global i32
 // PIE-DIRECT-NOPLT-NEXT: @weak_bar 

[clang] dec1bbb - Fix -allow-deprecated-dag-overlap in test/CodeGen/dso-local-executable.c

[Fangrui Song via cfe-commits]

Author: Fangrui Song
Date: 2020-12-03T21:24:38-08:00
New Revision: dec1bbb47cda3098c1621f780f10cee3fd91e7b1

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

LOG: Fix -allow-deprecated-dag-overlap in test/CodeGen/dso-local-executable.c

Added: 


Modified: 
clang/test/CodeGen/dso-local-executable.c

Removed: 




diff  --git a/clang/test/CodeGen/dso-local-executable.c 
b/clang/test/CodeGen/dso-local-executable.c
index 13e11158300f..3e1dcd209d04 100644
--- a/clang/test/CodeGen/dso-local-executable.c
+++ b/clang/test/CodeGen/dso-local-executable.c
@@ -1,93 +1,93 @@
-// RUN: %clang_cc1 -triple x86_64-pc-win32 -emit-llvm %s -o - | FileCheck 
-allow-deprecated-dag-overlap --check-prefix=COFF %s
-// COFF-DAG: @bar = external dso_local global i32
-// COFF-DAG: @weak_bar = extern_weak global i32
+// RUN: %clang_cc1 -triple x86_64-pc-win32 -emit-llvm %s -o - | FileCheck 
--check-prefix=COFF %s
+// COFF:  @baz = dso_local global i32 42
+// COFF-NEXT: @import_var = external dllimport global i32
+// COFF-NEXT: @weak_bar = extern_weak global i32
+// COFF-NEXT: @bar = external dso_local global i32
+// COFF-NEXT: @local_thread_var = dso_local thread_local global i32 42
+// COFF-NEXT: @thread_var = external dso_local thread_local global i32
 // COFF-DAG: declare dso_local void @foo()
-// COFF-DAG: @baz = dso_local global i32 42
 // COFF-DAG: define dso_local i32* @zed()
-// COFF-DAG: @thread_var = external dso_local thread_local global i32
-// COFF-DAG: @local_thread_var = dso_local thread_local global i32 42
-// COFF-DAG: @import_var = external dllimport global i32
 // COFF-DAG: declare dllimport void @import_func()
 
-// RUN: %clang_cc1 -triple x86_64-w64-mingw32 -emit-llvm %s -o - | FileCheck 
-allow-deprecated-dag-overlap --check-prefix=MINGW %s
-// MINGW-DAG: @bar = external global i32
-// MINGW-DAG: @weak_bar = extern_weak global i32
+// RUN: %clang_cc1 -triple x86_64-w64-mingw32 -emit-llvm %s -o - | FileCheck 
--check-prefix=MINGW %s
+// MINGW:  @baz = dso_local global i32 42
+// MINGW-NEXT: @import_var = external dllimport global i32
+// MINGW-NEXT: @weak_bar = extern_weak global i32
+// MINGW-NEXT: @bar = external global i32
+// MINGW-NEXT: @local_thread_var = dso_local thread_local global i32 42
+// MINGW-NEXT: @thread_var = external dso_local thread_local global i32
 // MINGW-DAG: declare dso_local void @foo()
-// MINGW-DAG: @baz = dso_local global i32 42
 // MINGW-DAG: define dso_local i32* @zed()
-// MINGW-DAG: @thread_var = external dso_local thread_local global i32
-// MINGW-DAG: @local_thread_var = dso_local thread_local global i32 42
-// MINGW-DAG: @import_var = external dllimport global i32
 // MINGW-DAG: declare dllimport void @import_func()
 
-// RUN: %clang_cc1 -triple x86_64-pc-linux -emit-llvm -mrelocation-model 
static %s -o - | FileCheck -allow-deprecated-dag-overlap --check-prefix=STATIC 
%s
-// STATIC-DAG: @bar = external dso_local global i32
-// STATIC-DAG: @weak_bar = extern_weak dso_local global i32
+// RUN: %clang_cc1 -triple x86_64 -emit-llvm -mrelocation-model static %s -o - 
| FileCheck --check-prefix=STATIC %s
+// STATIC:  @baz = dso_local global i32 42
+// STATIC-NEXT: @import_var = external dso_local global i32
+// STATIC-NEXT: @weak_bar = extern_weak dso_local global i32
+// STATIC-NEXT: @bar = external dso_local global i32
+// STATIC-NEXT: @local_thread_var = dso_local thread_local global i32 42
+// STATIC-NEXT: @thread_var = external thread_local global i32
 // STATIC-DAG: declare dso_local void @foo()
-// STATIC-DAG: @baz = dso_local global i32 42
 // STATIC-DAG: define dso_local i32* @zed()
-// STATIC-DAG: @thread_var = external thread_local global i32
-// STATIC-DAG: @local_thread_var = dso_local thread_local global i32 42
-// STATIC-DAG: @import_var = external dso_local global i32
 // STATIC-DAG: declare dso_local void @import_func()
 
-// RUN: %clang_cc1 -triple x86_64-pc-linux -emit-llvm -pic-is-pie 
-mpie-copy-relocations %s -o - | FileCheck -allow-deprecated-dag-overlap 
--check-prefix=PIE-COPY %s
-// PIE-COPY-DAG: @bar = external dso_local global i32
-// PIE-COPY-DAG: @weak_bar = extern_weak global i32
-// PIE-COPY-DAG: declare void @foo()
-// PIE-COPY-DAG: @baz = dso_local global i32 42
-// PIE-COPY-DAG: define dso_local i32* @zed()
-// PIE-COPY-DAG: @thread_var = external thread_local global i32
-// PIE-COPY-DAG: @local_thread_var = dso_local thread_local global i32 42
-// PIE-COPY-DAG: @import_var = external dso_local global i32
-// PIE-COPY-DAG: declare void @import_func()
-
-// RUN: %clang_cc1 -triple x86_64-pc-linux -emit-llvm -pic-is-pie %s -o - | 
FileCheck -allow-deprecated-dag-overlap --check-prefix=PIE %s
-// PIE-DAG: @bar = external global i32
-// PIE-DAG: @weak_bar = extern_weak global i32
+// RUN: %clang_cc1 -triple 

[clang] c4af1c8 - PR48383: Disallow decltype(auto) in pseudodestructor calls

[David Blaikie via cfe-commits]

Author: David Blaikie
Date: 2020-12-03T20:41:06-08:00
New Revision: c4af1c8d939b21ac7deb631887fc26db7451c592

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

LOG: PR48383: Disallow decltype(auto) in pseudodestructor calls

Added: 


Modified: 
clang/lib/Sema/SemaExprCXX.cpp
clang/test/SemaCXX/cxx1y-deduced-return-type.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
index e36d9adfbaba..241b8f72c56e 100644
--- a/clang/lib/Sema/SemaExprCXX.cpp
+++ b/clang/lib/Sema/SemaExprCXX.cpp
@@ -7595,6 +7595,11 @@ ExprResult Sema::ActOnPseudoDestructorExpr(Scope *S, 
Expr *Base,
   if (CheckArrow(*this, ObjectType, Base, OpKind, OpLoc))
 return ExprError();
 
+  if (DS.getTypeSpecType() == DeclSpec::TST_decltype_auto) {
+Diag(DS.getTypeSpecTypeLoc(), diag::err_decltype_auto_invalid);
+return true;
+  }
+
   QualType T = BuildDecltypeType(DS.getRepAsExpr(), DS.getTypeSpecTypeLoc(),
  false);
 

diff  --git a/clang/test/SemaCXX/cxx1y-deduced-return-type.cpp 
b/clang/test/SemaCXX/cxx1y-deduced-return-type.cpp
index 3e544c300884..051b0e7b1469 100644
--- a/clang/test/SemaCXX/cxx1y-deduced-return-type.cpp
+++ b/clang/test/SemaCXX/cxx1y-deduced-return-type.cpp
@@ -421,6 +421,7 @@ namespace DecltypeAutoShouldNotBeADecltypeSpecifier {
   namespace Dtor {
 struct A {};
 void f(A a) { a.~decltype(auto)(); } // expected-error {{'decltype(auto)' 
not allowed here}}
+void g(int i) { i.~decltype(auto)(); } // expected-error 
{{'decltype(auto)' not allowed here}}
   }
 
   namespace BaseClass {



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

[PATCH] D92361: [trivial-abi] Support types without a copy or move constructor.

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

Quuxplusone added a comment.

> You're right; thinking about it in the context of four value operations is 
> helpful.

FWIW, I think dragging "value operations" into the mix is exactly wrong (and 
referring to "destructive move" is extra wrong, in the specific context of 
C++). For a C++ object, we do have language-level operations for "original 
initialization, copy, and destruction" (I'm willing to grant that "move" is 
just a special form of "copy"); but we don't have "destructive move," neither 
at the language level nor at the ABI level. `[[trivial_abi]]` isn't about 
//language//-level operations at all. It's about granting the compiler the 
freedom to //make up and insert// bitwise-teleportations of the object from 
point A to point B, //even when the language spec says no operation is 
happening.// So for example, in C++, if we have `struct T { T() { print(this); 
} }; void f(T a) { print(); }`, and we do `f(T())`, there is no "copy" 
happening — from C++'s point of view, there is only one `T` constructor called. 
However, the pointer value printed inside `T()` is permitted to be 
//different// from the pointer value printed inside `f` — basically, after the 
`T` object is "constructed," it gets bitwise-teleported to a different location 
in memory, //without being copied, moved, or anything else// at the language 
level. https://godbolt.org/z/e6hzPP

You can see that the code still compiles even when `T`'s copy and move 
operations are deleted. https://godbolt.org/z/hfr14v This is because those 
operations //are not happening//, at the C++ language level. As far as C++ is 
concerned, we've got just one `T` object. We're not 
destroying-it-and-moving-it-elsewhere; that'd be the "relocation" operation 
described in my P1144  and D50119 
. A `[[trivially_relocatable]]` type permits 
the compiler to //replace// single pairs of "copy" and "destroy" operations 
with "bitwise-teleports." A `[[trivial_abi]]` type, on the other hand, permits 
the compiler to act at the //ABI// level and //insert// arbitrarily many 
"bitwise-teleport" operations even when the language spec calls for the object 
to remain in one place.

FWIW, (1) I doubt there's any reason to permit immovable objects to be 
`[[trivial_abi]]`; (2) I wish this attribute worked more like D50119 
 `[[trivially_relocatable]]` and less like 
D50119  `[[maybe_trivially_relocatable]]` (but 
I know @rjmccall feels exactly the opposite way). IMO, if the attribute simply 
meant "yes this thing is trivial for purposes of ABI and you can 
bitwise-teleport it whenever you like, full stop," then you wouldn't need to 
have these intricate discussions on precisely how triviality should be 
inherited in different situations — it would all "just work," as naturally as 
`[[trivially_relocatable]]`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92361

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

[PATCH] D92617: [DWARF] Allow toolchain to adjust specified DWARF version.

[David Blaikie via Phabricator via cfe-commits]

dblaikie added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3957
+  RenderDebugEnablingArgs(Args, CmdArgs, DebugInfoKind,
+  std::min(DWARFVersion, TC.getMaxDwarfVersion()),
   DebuggerTuning);

tra wrote:
> dblaikie wrote:
> > I think, ideally, the DWARFVersion calculation would happen up `if 
> > (EmitDwarf) {` block where all default and explicit dwarf version 
> > calculations are done.
> > 
> > I guess it's not done that way because of the gembed-source error path? 
> That's part of it. `-gembed-source` does need to know both the specified 
> DWARF version and the clamped one. I could move the calculation of the 
> effective DWARF version back to where it was in the first version of the 
> patch, and we'll need both the specified and the clamped values.
> That would bring back the need to explain which of the two DWARF versions to 
> use, where, and why. In the current revision the impact on DWARF version 
> clamping is localized to where it makes a difference. Neither is perfect, but 
> the current version is a bit easier to explain, I think.
> 
> As for moving it under `if( EmitDwarf)`, the problem is that the DWARF 
> version also affects cases when `EmitDwarf` is false, so the clamping needs 
> to be done regardless. E.g. the `-gmlt` option that was used in our build 
> break that uncovered the issue. We do not emit DWARF per se, but the 
> `DefaultDWARFVersion` does affect the generation of the lineinfo debugging 
> and we do need to clamp it.
> 
> 
Is the warning necessary? Could we let -gembed-source be an error if you're 
targeting NVPTX? (I realize that goes a bit against the idea of this patch, 
which is to keep DWARFv2 even when you've got a bunch of DWARFv5 flags that you 
presumably want for the rest of your builds)

Or, I guess another option, might be to make -gembed-source a warning for 
non-v5 DWARF in general, instead of an error, then the implementation could be 
generic.

What I'd really like is for the DWARFVersion to be computed as compactly as 
possible without the concept of multiple versions leaking out as much as 
possible.

> As for moving it under if( EmitDwarf), the problem is that the DWARF version 
> also affects cases when EmitDwarf is false, so the clamping needs to be done 
> regardless. E.g. the -gmlt option that was used in our build break that 
> uncovered the issue.

That doesn't sound right to me, -gmlt qualifies as "emitting DWARF" and at 
least a quick/simple debug through the driver for "clang x.cpp -gmlt" does seem 
to lead to "EmitDwarf" being true and the block at line 3841 being reached. 
EmitDwarf is set to true in that case at 3834 because DebugInfoKind is not 
NoDebugInfo (it's DebugLineTablesOnly)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92617

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

[PATCH] D92617: [DWARF] Allow toolchain to adjust specified DWARF version.

[Artem Belevich via Phabricator via cfe-commits]

tra added inline comments.



Comment at: clang/test/Driver/cuda-unsupported-debug-options.cu:22
+// Make sure we do not see any dwarf version other than 2, regardless of 
what's used on the host side.
+// CHECK-NOT: {{-dwarf-version=[^2]}}
 // CHECK: "-triple" "x86_64

MaskRay wrote:
> A NOT pattern may easily become stale and do not actually check anything. 
> Just turn to a positive pattern?
In this case the issue is with the CHECK-NOT line above. I'll have to replicate 
it around the positive match of `-dwarf-version` which would raise more 
questions.

I wish filecheck would allow to `mark` a region and then run multiple matches 
on it, instead of re-anchoring on each match. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92617

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

[PATCH] D92617: [DWARF] Allow toolchain to adjust specified DWARF version.

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clang/test/Driver/cuda-unsupported-debug-options.cu:22
+// Make sure we do not see any dwarf version other than 2, regardless of 
what's used on the host side.
+// CHECK-NOT: {{-dwarf-version=[^2]}}
 // CHECK: "-triple" "x86_64

A NOT pattern may easily become stale and do not actually check anything. Just 
turn to a positive pattern?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92617

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

[PATCH] D92617: [DWARF] Allow toolchain to adjust specified DWARF version.

[Artem Belevich via Phabricator via cfe-commits]

tra added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3957
+  RenderDebugEnablingArgs(Args, CmdArgs, DebugInfoKind,
+  std::min(DWARFVersion, TC.getMaxDwarfVersion()),
   DebuggerTuning);

dblaikie wrote:
> I think, ideally, the DWARFVersion calculation would happen up `if 
> (EmitDwarf) {` block where all default and explicit dwarf version 
> calculations are done.
> 
> I guess it's not done that way because of the gembed-source error path? 
That's part of it. `-gembed-source` does need to know both the specified DWARF 
version and the clamped one. I could move the calculation of the effective 
DWARF version back to where it was in the first version of the patch, and we'll 
need both the specified and the clamped values.
That would bring back the need to explain which of the two DWARF versions to 
use, where, and why. In the current revision the impact on DWARF version 
clamping is localized to where it makes a difference. Neither is perfect, but 
the current version is a bit easier to explain, I think.

As for moving it under `if( EmitDwarf)`, the problem is that the DWARF version 
also affects cases when `EmitDwarf` is false, so the clamping needs to be done 
regardless. E.g. the `-gmlt` option that was used in our build break that 
uncovered the issue. We do not emit DWARF per se, but the `DefaultDWARFVersion` 
does affect the generation of the lineinfo debugging and we do need to clamp it.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92617

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

[PATCH] D92544: [NFC] [Clang] Fix ppc64le vaarg OpenMP test in CodeGen

[Qiu Chaofan via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rG9378a366b2b2: [NFC] [Clang] Fix ppc64le vaarg OpenMP test in 
CodeGen (authored by qiucf).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Changed prior to commit:
  https://reviews.llvm.org/D92544?vs=309161=309446#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92544

Files:
  clang/test/CodeGen/ppc64le-varargs-f128.c
  clang/test/Driver/ppc-openmp-f128.c


Index: clang/test/Driver/ppc-openmp-f128.c
===
--- clang/test/Driver/ppc-openmp-f128.c
+++ /dev/null
@@ -1,39 +0,0 @@
-// RUN: %clang -target powerpc64le-unknown-linux-gnu -S -emit-llvm \
-// RUN:   -fopenmp-targets=ppc64le -mfloat128 -mabi=ieeelongdouble -mcpu=pwr9 \
-// RUN:   -Xopenmp-target=ppc64le -mcpu=pwr9 -Xopenmp-target=ppc64le \
-// RUN:   -mfloat128 -fopenmp=libomp -o - %s | FileCheck %s -check-prefix=OMP
-
-#include 
-
-void foo_ld(long double);
-void foo_fq(__float128);
-
-// Verify cases when OpenMP target's and host's long-double semantics differ.
-
-// OMP-LABEL: define internal void @.omp_outlined.
-// OMP: %[[CUR:[0-9a-zA-Z_.]+]] = load i8*, i8**
-// OMP: %[[V2:[0-9a-zA-Z_.]+]] = bitcast i8* %[[CUR]] to ppc_fp128*
-// OMP: %[[V3:[0-9a-zA-Z_.]+]] = load ppc_fp128, ppc_fp128* %[[V2]], align 8
-// OMP: call void @foo_ld(ppc_fp128 %[[V3]])
-
-// OMP-LABEL: define dso_local void @omp
-// OMP: %[[AP1:[0-9a-zA-Z_.]+]] = bitcast i8** %[[AP:[0-9a-zA-Z_.]+]] to i8*
-// OMP: call void @llvm.va_start(i8* %[[AP1]])
-// OMP: %[[CUR:[0-9a-zA-Z_.]+]] = load i8*, i8** %[[AP]], align 8
-// OMP: %[[V0:[0-9a-zA-Z_.]+]] = ptrtoint i8* %[[CUR]] to i64
-// OMP: %[[V1:[0-9a-zA-Z_.]+]] = add i64 %[[V0]], 15
-// OMP: %[[V2:[0-9a-zA-Z_.]+]] = and i64 %[[V1]], -16
-// OMP: %[[ALIGN:[0-9a-zA-Z_.]+]] = inttoptr i64 %[[V2]] to i8*
-// OMP: %[[V3:[0-9a-zA-Z_.]+]] = bitcast i8* %[[ALIGN]] to fp128*
-// OMP: %[[V4:[0-9a-zA-Z_.]+]] = load fp128, fp128* %[[V3]], align 16
-// OMP: call void @foo_ld(fp128 %[[V4]])
-void omp(int n, ...) {
-  va_list ap;
-  va_start(ap, n);
-  foo_ld(va_arg(ap, long double));
-  #pragma omp target parallel
-  for (int i = 1; i < n; ++i) {
-foo_ld(va_arg(ap, long double));
-  }
-  va_end(ap);
-}
Index: clang/test/CodeGen/ppc64le-varargs-f128.c
===
--- clang/test/CodeGen/ppc64le-varargs-f128.c
+++ clang/test/CodeGen/ppc64le-varargs-f128.c
@@ -5,11 +5,51 @@
 // RUN:   -target-cpu pwr9 -target-feature +float128 \
 // RUN:   -o - %s | FileCheck %s -check-prefix=IBM
 
+// RUN: %clang_cc1 -triple ppc64le -emit-llvm-bc %s -target-cpu pwr9 \
+// RUN:   -target-feature +float128 -mabi=ieeelongdouble -fopenmp \
+// RUN:   -fopenmp-targets=ppc64le -o %t-ppc-host.bc
+// RUN: %clang_cc1 -triple ppc64le -aux-triple ppc64le %s -target-cpu pwr9 \
+// RUN:   -target-feature +float128 -fopenmp -fopenmp-is-device -emit-llvm \
+// RUN:   -fopenmp-host-ir-file-path %t-ppc-host.bc -o - | FileCheck %s \
+// RUN:   -check-prefix=OMP-TARGET
+// RUN: %clang_cc1 %t-ppc-host.bc -emit-llvm -o - | FileCheck %s \
+// RUN:   -check-prefix=OMP-HOST
+
 #include 
 
 void foo_ld(long double);
 void foo_fq(__float128);
 
+// Verify cases when OpenMP target's and host's long-double semantics differ.
+
+// OMP-TARGET-LABEL: define internal void @.omp_outlined.(
+// OMP-TARGET: %[[CUR:[0-9a-zA-Z_.]+]] = load i8*, i8**
+// OMP-TARGET: %[[V2:[0-9a-zA-Z_.]+]] = bitcast i8* %[[CUR]] to ppc_fp128*
+// OMP-TARGET: %[[V3:[0-9a-zA-Z_.]+]] = load ppc_fp128, ppc_fp128* %[[V2]], 
align 8
+// OMP-TARGET: call void @foo_ld(ppc_fp128 %[[V3]])
+
+// OMP-HOST-LABEL: define void @omp(
+// OMP-HOST: %[[AP1:[0-9a-zA-Z_.]+]] = bitcast i8** %[[AP:[0-9a-zA-Z_.]+]] to 
i8*
+// OMP-HOST: call void @llvm.va_start(i8* %[[AP1]])
+// OMP-HOST: %[[CUR:[0-9a-zA-Z_.]+]] = load i8*, i8** %[[AP]], align 8
+// OMP-HOST: %[[V0:[0-9a-zA-Z_.]+]] = ptrtoint i8* %[[CUR]] to i64
+// OMP-HOST: %[[V1:[0-9a-zA-Z_.]+]] = add i64 %[[V0]], 15
+// OMP-HOST: %[[V2:[0-9a-zA-Z_.]+]] = and i64 %[[V1]], -16
+// OMP-HOST: %[[ALIGN:[0-9a-zA-Z_.]+]] = inttoptr i64 %[[V2]] to i8*
+// OMP-HOST: %[[V3:[0-9a-zA-Z_.]+]] = bitcast i8* %[[ALIGN]] to fp128*
+// OMP-HOST: %[[V4:[0-9a-zA-Z_.]+]] = load fp128, fp128* %[[V3]], align 16
+// OMP-HOST: call void @foo_ld(fp128 %[[V4]])
+void omp(int n, ...) {
+  va_list ap;
+  va_start(ap, n);
+  foo_ld(va_arg(ap, long double));
+  #pragma omp target parallel
+  for (int i = 1; i < n; ++i) {
+foo_ld(va_arg(ap, long double));
+  }
+  va_end(ap);
+}
+
 // IEEE-LABEL: define void @f128
 // IEEE: %[[AP1:[0-9a-zA-Z_.]+]] = bitcast i8** %[[AP:[0-9a-zA-Z_.]+]] to i8*
 // IEEE: call void @llvm.va_start(i8* %[[AP1]])


Index: clang/test/Driver/ppc-openmp-f128.c

[clang] 9378a36 - [NFC] [Clang] Fix ppc64le vaarg OpenMP test in CodeGen

[Qiu Chaofan via cfe-commits]

Author: Qiu Chaofan
Date: 2020-12-04T11:29:55+08:00
New Revision: 9378a366b2b256ebd1b2763141f683ab9b48c303

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

LOG: [NFC] [Clang] Fix ppc64le vaarg OpenMP test in CodeGen

Reviewed By: MaskRay

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

Added: 


Modified: 
clang/test/CodeGen/ppc64le-varargs-f128.c

Removed: 
clang/test/Driver/ppc-openmp-f128.c



diff  --git a/clang/test/CodeGen/ppc64le-varargs-f128.c 
b/clang/test/CodeGen/ppc64le-varargs-f128.c
index 0b085859c5ac..7868fe322ce8 100644
--- a/clang/test/CodeGen/ppc64le-varargs-f128.c
+++ b/clang/test/CodeGen/ppc64le-varargs-f128.c
@@ -5,11 +5,51 @@
 // RUN:   -target-cpu pwr9 -target-feature +float128 \
 // RUN:   -o - %s | FileCheck %s -check-prefix=IBM
 
+// RUN: %clang_cc1 -triple ppc64le -emit-llvm-bc %s -target-cpu pwr9 \
+// RUN:   -target-feature +float128 -mabi=ieeelongdouble -fopenmp \
+// RUN:   -fopenmp-targets=ppc64le -o %t-ppc-host.bc
+// RUN: %clang_cc1 -triple ppc64le -aux-triple ppc64le %s -target-cpu pwr9 \
+// RUN:   -target-feature +float128 -fopenmp -fopenmp-is-device -emit-llvm \
+// RUN:   -fopenmp-host-ir-file-path %t-ppc-host.bc -o - | FileCheck %s \
+// RUN:   -check-prefix=OMP-TARGET
+// RUN: %clang_cc1 %t-ppc-host.bc -emit-llvm -o - | FileCheck %s \
+// RUN:   -check-prefix=OMP-HOST
+
 #include 
 
 void foo_ld(long double);
 void foo_fq(__float128);
 
+// Verify cases when OpenMP target's and host's long-double semantics 
diff er.
+
+// OMP-TARGET-LABEL: define internal void @.omp_outlined.(
+// OMP-TARGET: %[[CUR:[0-9a-zA-Z_.]+]] = load i8*, i8**
+// OMP-TARGET: %[[V2:[0-9a-zA-Z_.]+]] = bitcast i8* %[[CUR]] to ppc_fp128*
+// OMP-TARGET: %[[V3:[0-9a-zA-Z_.]+]] = load ppc_fp128, ppc_fp128* %[[V2]], 
align 8
+// OMP-TARGET: call void @foo_ld(ppc_fp128 %[[V3]])
+
+// OMP-HOST-LABEL: define void @omp(
+// OMP-HOST: %[[AP1:[0-9a-zA-Z_.]+]] = bitcast i8** %[[AP:[0-9a-zA-Z_.]+]] to 
i8*
+// OMP-HOST: call void @llvm.va_start(i8* %[[AP1]])
+// OMP-HOST: %[[CUR:[0-9a-zA-Z_.]+]] = load i8*, i8** %[[AP]], align 8
+// OMP-HOST: %[[V0:[0-9a-zA-Z_.]+]] = ptrtoint i8* %[[CUR]] to i64
+// OMP-HOST: %[[V1:[0-9a-zA-Z_.]+]] = add i64 %[[V0]], 15
+// OMP-HOST: %[[V2:[0-9a-zA-Z_.]+]] = and i64 %[[V1]], -16
+// OMP-HOST: %[[ALIGN:[0-9a-zA-Z_.]+]] = inttoptr i64 %[[V2]] to i8*
+// OMP-HOST: %[[V3:[0-9a-zA-Z_.]+]] = bitcast i8* %[[ALIGN]] to fp128*
+// OMP-HOST: %[[V4:[0-9a-zA-Z_.]+]] = load fp128, fp128* %[[V3]], align 16
+// OMP-HOST: call void @foo_ld(fp128 %[[V4]])
+void omp(int n, ...) {
+  va_list ap;
+  va_start(ap, n);
+  foo_ld(va_arg(ap, long double));
+  #pragma omp target parallel
+  for (int i = 1; i < n; ++i) {
+foo_ld(va_arg(ap, long double));
+  }
+  va_end(ap);
+}
+
 // IEEE-LABEL: define void @f128
 // IEEE: %[[AP1:[0-9a-zA-Z_.]+]] = bitcast i8** %[[AP:[0-9a-zA-Z_.]+]] to i8*
 // IEEE: call void @llvm.va_start(i8* %[[AP1]])

diff  --git a/clang/test/Driver/ppc-openmp-f128.c 
b/clang/test/Driver/ppc-openmp-f128.c
deleted file mode 100644
index bff6fe35e526..
--- a/clang/test/Driver/ppc-openmp-f128.c
+++ /dev/null
@@ -1,39 +0,0 @@
-// RUN: %clang -target powerpc64le-unknown-linux-gnu -S -emit-llvm \
-// RUN:   -fopenmp-targets=ppc64le -mfloat128 -mabi=ieeelongdouble -mcpu=pwr9 \
-// RUN:   -Xopenmp-target=ppc64le -mcpu=pwr9 -Xopenmp-target=ppc64le \
-// RUN:   -mfloat128 -fopenmp=libomp -o - %s | FileCheck %s -check-prefix=OMP
-
-#include 
-
-void foo_ld(long double);
-void foo_fq(__float128);
-
-// Verify cases when OpenMP target's and host's long-double semantics 
diff er.
-
-// OMP-LABEL: define internal void @.omp_outlined.
-// OMP: %[[CUR:[0-9a-zA-Z_.]+]] = load i8*, i8**
-// OMP: %[[V2:[0-9a-zA-Z_.]+]] = bitcast i8* %[[CUR]] to ppc_fp128*
-// OMP: %[[V3:[0-9a-zA-Z_.]+]] = load ppc_fp128, ppc_fp128* %[[V2]], align 8
-// OMP: call void @foo_ld(ppc_fp128 %[[V3]])
-
-// OMP-LABEL: define dso_local void @omp
-// OMP: %[[AP1:[0-9a-zA-Z_.]+]] = bitcast i8** %[[AP:[0-9a-zA-Z_.]+]] to i8*
-// OMP: call void @llvm.va_start(i8* %[[AP1]])
-// OMP: %[[CUR:[0-9a-zA-Z_.]+]] = load i8*, i8** %[[AP]], align 8
-// OMP: %[[V0:[0-9a-zA-Z_.]+]] = ptrtoint i8* %[[CUR]] to i64
-// OMP: %[[V1:[0-9a-zA-Z_.]+]] = add i64 %[[V0]], 15
-// OMP: %[[V2:[0-9a-zA-Z_.]+]] = and i64 %[[V1]], -16
-// OMP: %[[ALIGN:[0-9a-zA-Z_.]+]] = inttoptr i64 %[[V2]] to i8*
-// OMP: %[[V3:[0-9a-zA-Z_.]+]] = bitcast i8* %[[ALIGN]] to fp128*
-// OMP: %[[V4:[0-9a-zA-Z_.]+]] = load fp128, fp128* %[[V3]], align 16
-// OMP: call void @foo_ld(fp128 %[[V4]])
-void omp(int n, ...) {
-  va_list ap;
-  va_start(ap, n);
-  foo_ld(va_arg(ap, long double));
-  #pragma omp target parallel
-  for (int i = 1; i < n; ++i) {
-foo_ld(va_arg(ap, long double));
-  }
-  va_end(ap);
-}

[PATCH] D92080: [Clang] Mutate long-double math builtins into f128 under IEEE-quad

[Qiu Chaofan via Phabricator via cfe-commits]

qiucf added a comment.

Any comments...? I don't have knowledge if there's any plan about implementing 
`*f128` math functions on X86 libcs. But as I see so far, `*l` doesn't work 
well on `fp128`. Keeping this target independent currently seems reasonable to 
me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92080

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

[PATCH] D92628: [HIP] Fix bug in driver about wavefront size

[Matt Arsenault via Phabricator via cfe-commits]

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

LGTM although I thought there were existing test files for this


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

https://reviews.llvm.org/D92628

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

[PATCH] D92630: ARCMigrate: Use hash_combine in the DenseMapInfo for EditEntry

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith created this revision.
dexonsmith added reviewers: JDevlieghere, jansvoboda11, arphaman.
Herald added a subscriber: ributzka.
dexonsmith requested review of this revision.
Herald added a project: clang.

Simplify the DenseMapInfo for `EditEntry` by migrating from
`FoldingSetNodeID` to `llvm::hash_combine`. Besides the cleanup, this
reduces the diff for a future patch which changes the type of one of the
fields.

There should be no real functionality change here, although I imagine
the hash value will churn since its a different hashing infrastructure.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D92630

Files:
  clang/lib/ARCMigrate/ObjCMT.cpp


Index: clang/lib/ARCMigrate/ObjCMT.cpp
===
--- clang/lib/ARCMigrate/ObjCMT.cpp
+++ clang/lib/ARCMigrate/ObjCMT.cpp
@@ -2054,12 +2054,8 @@
 return Entry;
   }
   static unsigned getHashValue(const EditEntry& Val) {
-llvm::FoldingSetNodeID ID;
-ID.AddPointer(Val.File);
-ID.AddInteger(Val.Offset);
-ID.AddInteger(Val.RemoveLen);
-ID.AddString(Val.Text);
-return ID.ComputeHash();
+return (unsigned)llvm::hash_combine(Val.File, Val.Offset, Val.RemoveLen,
+Val.Text);
   }
   static bool isEqual(const EditEntry , const EditEntry ) {
 return LHS.File == RHS.File &&


Index: clang/lib/ARCMigrate/ObjCMT.cpp
===
--- clang/lib/ARCMigrate/ObjCMT.cpp
+++ clang/lib/ARCMigrate/ObjCMT.cpp
@@ -2054,12 +2054,8 @@
 return Entry;
   }
   static unsigned getHashValue(const EditEntry& Val) {
-llvm::FoldingSetNodeID ID;
-ID.AddPointer(Val.File);
-ID.AddInteger(Val.Offset);
-ID.AddInteger(Val.RemoveLen);
-ID.AddString(Val.Text);
-return ID.ComputeHash();
+return (unsigned)llvm::hash_combine(Val.File, Val.Offset, Val.RemoveLen,
+Val.Text);
   }
   static bool isEqual(const EditEntry , const EditEntry ) {
 return LHS.File == RHS.File &&
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D92628: [HIP] Fix bug in driver about wavefront size

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl created this revision.
yaxunl added reviewers: tra, arsenm.
Herald added subscribers: kerbowa, nhaehnle, jvesely.
yaxunl requested review of this revision.
Herald added a subscriber: wdng.

The static variable causes it only initialized once and take
the same value for different GPU archs, whereas they
may be different for different GPU archs, e.g. when
there are both gfx900 and gfx1010.

Removing `static` fixes that.


https://reviews.llvm.org/D92628

Files:
  clang/lib/Driver/ToolChains/AMDGPU.cpp
  clang/test/Driver/hip-wavefront-size.hip


Index: clang/test/Driver/hip-wavefront-size.hip
===
--- /dev/null
+++ clang/test/Driver/hip-wavefront-size.hip
@@ -0,0 +1,21 @@
+// REQUIRES: clang-driver,amdgpu-registered-target
+
+// RUN: %clang -### -target x86_64-linux-gnu \
+// RUN:   --cuda-gpu-arch=gfx900 \
+// RUN:   --rocm-path=%S/Inputs/rocm --cuda-device-only %s \
+// RUN:   2>&1 | FileCheck %s --check-prefixes=WAVE64
+// WAVE64: "-mlink-builtin-bitcode" "{{.*}}oclc_wavefrontsize64_on.bc"{{.*}} 
"-target-cpu" "gfx900"
+
+// RUN: %clang -### -target x86_64-linux-gnu \
+// RUN:   --cuda-gpu-arch=gfx1010 \
+// RUN:   --rocm-path=%S/Inputs/rocm --cuda-device-only %s \
+// RUN:   2>&1 | FileCheck %s --check-prefixes=WAVE32
+// WAVE32: "-mlink-builtin-bitcode" "{{.*}}oclc_wavefrontsize64_off.bc"{{.*}} 
"-target-cpu" "gfx1010"
+
+// RUN: %clang -### -target x86_64-linux-gnu \
+// RUN:   --cuda-gpu-arch=gfx1010 \
+// RUN:   --cuda-gpu-arch=gfx900 \
+// RUN:   --rocm-path=%S/Inputs/rocm --cuda-device-only %s \
+// RUN:   2>&1 | FileCheck %s --check-prefixes=BOTH
+// BOTH-DAG: "-mlink-builtin-bitcode" "{{.*}}oclc_wavefrontsize64_on.bc"{{.*}} 
"-target-cpu" "gfx900"
+// BOTH-DAG: "-mlink-builtin-bitcode" 
"{{.*}}oclc_wavefrontsize64_off.bc"{{.*}} "-target-cpu" "gfx1010"
Index: clang/lib/Driver/ToolChains/AMDGPU.cpp
===
--- clang/lib/Driver/ToolChains/AMDGPU.cpp
+++ clang/lib/Driver/ToolChains/AMDGPU.cpp
@@ -499,7 +499,7 @@
 bool AMDGPUToolChain::isWave64(const llvm::opt::ArgList ,
llvm::AMDGPU::GPUKind Kind) {
   const unsigned ArchAttr = llvm::AMDGPU::getArchAttrAMDGCN(Kind);
-  static bool HasWave32 = (ArchAttr & llvm::AMDGPU::FEATURE_WAVE32);
+  bool HasWave32 = (ArchAttr & llvm::AMDGPU::FEATURE_WAVE32);
 
   return !HasWave32 || DriverArgs.hasFlag(
 options::OPT_mwavefrontsize64, options::OPT_mno_wavefrontsize64, false);


Index: clang/test/Driver/hip-wavefront-size.hip
===
--- /dev/null
+++ clang/test/Driver/hip-wavefront-size.hip
@@ -0,0 +1,21 @@
+// REQUIRES: clang-driver,amdgpu-registered-target
+
+// RUN: %clang -### -target x86_64-linux-gnu \
+// RUN:   --cuda-gpu-arch=gfx900 \
+// RUN:   --rocm-path=%S/Inputs/rocm --cuda-device-only %s \
+// RUN:   2>&1 | FileCheck %s --check-prefixes=WAVE64
+// WAVE64: "-mlink-builtin-bitcode" "{{.*}}oclc_wavefrontsize64_on.bc"{{.*}} "-target-cpu" "gfx900"
+
+// RUN: %clang -### -target x86_64-linux-gnu \
+// RUN:   --cuda-gpu-arch=gfx1010 \
+// RUN:   --rocm-path=%S/Inputs/rocm --cuda-device-only %s \
+// RUN:   2>&1 | FileCheck %s --check-prefixes=WAVE32
+// WAVE32: "-mlink-builtin-bitcode" "{{.*}}oclc_wavefrontsize64_off.bc"{{.*}} "-target-cpu" "gfx1010"
+
+// RUN: %clang -### -target x86_64-linux-gnu \
+// RUN:   --cuda-gpu-arch=gfx1010 \
+// RUN:   --cuda-gpu-arch=gfx900 \
+// RUN:   --rocm-path=%S/Inputs/rocm --cuda-device-only %s \
+// RUN:   2>&1 | FileCheck %s --check-prefixes=BOTH
+// BOTH-DAG: "-mlink-builtin-bitcode" "{{.*}}oclc_wavefrontsize64_on.bc"{{.*}} "-target-cpu" "gfx900"
+// BOTH-DAG: "-mlink-builtin-bitcode" "{{.*}}oclc_wavefrontsize64_off.bc"{{.*}} "-target-cpu" "gfx1010"
Index: clang/lib/Driver/ToolChains/AMDGPU.cpp
===
--- clang/lib/Driver/ToolChains/AMDGPU.cpp
+++ clang/lib/Driver/ToolChains/AMDGPU.cpp
@@ -499,7 +499,7 @@
 bool AMDGPUToolChain::isWave64(const llvm::opt::ArgList ,
llvm::AMDGPU::GPUKind Kind) {
   const unsigned ArchAttr = llvm::AMDGPU::getArchAttrAMDGCN(Kind);
-  static bool HasWave32 = (ArchAttr & llvm::AMDGPU::FEATURE_WAVE32);
+  bool HasWave32 = (ArchAttr & llvm::AMDGPU::FEATURE_WAVE32);
 
   return !HasWave32 || DriverArgs.hasFlag(
 options::OPT_mwavefrontsize64, options::OPT_mno_wavefrontsize64, false);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D92627: Basic: Add hashing support for FileEntryRef and DirectoryEntryRef

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith created this revision.
dexonsmith added reviewers: arphaman, JDevlieghere, jansvoboda11.
Herald added a subscriber: ributzka.
dexonsmith requested review of this revision.
Herald added a project: clang.

Allow hashing FileEntryRef and DirectoryEntryRef via `hash_value`, and
use that to implement `DenseMapInfo`. This hash be equal whenever the
entry is the same (the name used to reference it is not relevant).

Also added `DirectoryEntryRef::isSameRef` as a drive-by to facilitate
testing.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D92627

Files:
  clang/include/clang/Basic/DirectoryEntry.h
  clang/include/clang/Basic/FileEntry.h
  clang/unittests/Basic/FileEntryTest.cpp

Index: clang/unittests/Basic/FileEntryTest.cpp
===
--- clang/unittests/Basic/FileEntryTest.cpp
+++ clang/unittests/Basic/FileEntryTest.cpp
@@ -7,6 +7,7 @@
 //===--===//
 
 #include "clang/Basic/FileEntry.h"
+#include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/StringMap.h"
 #include "gtest/gtest.h"
 
@@ -22,11 +23,21 @@
   FileMap Files;
   DirMap Dirs;
 
-  DirectoryEntry D;
-  DirectoryEntryRef DR;
   SmallVector, 5> FEs;
+  SmallVector, 5> DEs;
+  DirectoryEntryRef DR;
 
-  RefMaps() : DR(*Dirs.insert({"dir", D}).first) {}
+  RefMaps() : DR(addDirectory("dir")) {}
+
+  DirectoryEntryRef addDirectory(StringRef Name) {
+DEs.push_back(std::make_unique());
+return DirectoryEntryRef(*Dirs.insert({Name, *DEs.back()}).first);
+  }
+  DirectoryEntryRef addDirectoryAlias(StringRef Name, DirectoryEntryRef Base) {
+return DirectoryEntryRef(
+*Dirs.insert({Name, const_cast(Base.getDirEntry())})
+ .first);
+  }
 
   FileEntryRef addFile(StringRef Name) {
 FEs.push_back(std::make_unique());
@@ -112,4 +123,74 @@
   EXPECT_FALSE(R1.isSameRef(R1Also));
 }
 
+TEST(FileEntryTest, DenseMapInfo) {
+  RefMaps Refs;
+  FileEntryRef R1 = Refs.addFile("1");
+  FileEntryRef R2 = Refs.addFile("2");
+  FileEntryRef R1Also = Refs.addFileAlias("1-also", R1);
+
+  // Insert R1Also first and confirm it "wins".
+  {
+SmallDenseSet Set;
+Set.insert(R1Also);
+Set.insert(R1);
+Set.insert(R2);
+EXPECT_TRUE(Set.find(R1Also)->isSameRef(R1Also));
+EXPECT_TRUE(Set.find(R1)->isSameRef(R1Also));
+EXPECT_TRUE(Set.find(R2)->isSameRef(R2));
+  }
+
+  // Insert R1Also second and confirm R1 "wins".
+  {
+SmallDenseSet Set;
+Set.insert(R1);
+Set.insert(R1Also);
+Set.insert(R2);
+EXPECT_TRUE(Set.find(R1Also)->isSameRef(R1));
+EXPECT_TRUE(Set.find(R1)->isSameRef(R1));
+EXPECT_TRUE(Set.find(R2)->isSameRef(R2));
+  }
+}
+
+TEST(DirectoryEntryTest, isSameRef) {
+  RefMaps Refs;
+  DirectoryEntryRef R1 = Refs.addDirectory("1");
+  DirectoryEntryRef R2 = Refs.addDirectory("2");
+  DirectoryEntryRef R1Also = Refs.addDirectoryAlias("1-also", R1);
+
+  EXPECT_TRUE(R1.isSameRef(DirectoryEntryRef(R1)));
+  EXPECT_TRUE(R1.isSameRef(DirectoryEntryRef(R1.getMapEntry(;
+  EXPECT_FALSE(R1.isSameRef(R2));
+  EXPECT_FALSE(R1.isSameRef(R1Also));
+}
+
+TEST(DirectoryEntryTest, DenseMapInfo) {
+  RefMaps Refs;
+  DirectoryEntryRef R1 = Refs.addDirectory("1");
+  DirectoryEntryRef R2 = Refs.addDirectory("2");
+  DirectoryEntryRef R1Also = Refs.addDirectoryAlias("1-also", R1);
+
+  // Insert R1Also first and confirm it "wins".
+  {
+SmallDenseSet Set;
+Set.insert(R1Also);
+Set.insert(R1);
+Set.insert(R2);
+EXPECT_TRUE(Set.find(R1Also)->isSameRef(R1Also));
+EXPECT_TRUE(Set.find(R1)->isSameRef(R1Also));
+EXPECT_TRUE(Set.find(R2)->isSameRef(R2));
+  }
+
+  // Insert R1Also second and confirm R1 "wins".
+  {
+SmallDenseSet Set;
+Set.insert(R1);
+Set.insert(R1Also);
+Set.insert(R2);
+EXPECT_TRUE(Set.find(R1Also)->isSameRef(R1));
+EXPECT_TRUE(Set.find(R1)->isSameRef(R1));
+EXPECT_TRUE(Set.find(R2)->isSameRef(R2));
+  }
+}
+
 } // end namespace
Index: clang/include/clang/Basic/FileEntry.h
===
--- clang/include/clang/Basic/FileEntry.h
+++ clang/include/clang/Basic/FileEntry.h
@@ -16,6 +16,8 @@
 
 #include "clang/Basic/DirectoryEntry.h"
 #include "clang/Basic/LLVM.h"
+#include "llvm/ADT/DenseMapInfo.h"
+#include "llvm/ADT/Hashing.h"
 #include "llvm/ADT/PointerUnion.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
@@ -88,6 +90,12 @@
 return !(LHS == RHS);
   }
 
+  /// Hash code is based on the FileEntry, not the specific named reference,
+  /// just like operator==.
+  friend llvm::hash_code hash_value(FileEntryRef Ref) {
+return llvm::hash_value(());
+  }
+
   struct MapValue;
 
   /// Type used in the StringMap.
@@ -154,6 +162,20 @@
   FileEntryRef(optional_none_tag) : ME(nullptr) {}
   bool hasOptionalValue() const { return ME; }
 
+  friend struct llvm::DenseMapInfo;
+  struct dense_map_empty_tag {};
+  struct 

[PATCH] D92078: [asan] Default to -asan-use-private-alias=1

[Fangrui Song via Phabricator via cfe-commits]

MaskRay updated this revision to Diff 309434.
MaskRay marked an inline comment as done.
MaskRay added a comment.

Improve odr-vtable.cpp


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92078

Files:
  clang/test/CodeGen/asan-globals-odr.cpp
  clang/test/CodeGen/asan-static-odr.cpp
  compiler-rt/test/asan/TestCases/Linux/odr-violation.cpp
  compiler-rt/test/asan/TestCases/Linux/odr-vtable.cpp
  compiler-rt/test/asan/TestCases/Linux/odr_c_test.c
  llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
  llvm/test/Instrumentation/AddressSanitizer/global_metadata.ll
  llvm/test/Instrumentation/AddressSanitizer/global_metadata_darwin.ll
  llvm/test/Instrumentation/AddressSanitizer/local_alias.ll
  llvm/test/Instrumentation/AddressSanitizer/odr-check-ignore.ll

Index: llvm/test/Instrumentation/AddressSanitizer/odr-check-ignore.ll
===
--- llvm/test/Instrumentation/AddressSanitizer/odr-check-ignore.ll
+++ llvm/test/Instrumentation/AddressSanitizer/odr-check-ignore.ll
@@ -1,5 +1,5 @@
-; RUN: opt < %s -asan -asan-module -enable-new-pm=0 -S | FileCheck %s 
-; RUN: opt < %s -passes='asan-pipeline' -S | FileCheck %s 
+; RUN: opt < %s -asan -asan-module -asan-use-private-alias=0 -enable-new-pm=0 -S | FileCheck %s
+; RUN: opt < %s -passes='asan-pipeline' -asan-use-private-alias=0 -S | FileCheck %s
 
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
Index: llvm/test/Instrumentation/AddressSanitizer/local_alias.ll
===
--- llvm/test/Instrumentation/AddressSanitizer/local_alias.ll
+++ llvm/test/Instrumentation/AddressSanitizer/local_alias.ll
@@ -1,11 +1,11 @@
-; RUN: opt < %s -asan -asan-module -enable-new-pm=0 -S | FileCheck %s --check-prefixes=CHECK-NOALIAS,CHECK-NOINDICATOR
-; RUN: opt < %s -passes='asan-pipeline' -S | FileCheck %s --check-prefixes=CHECK-NOALIAS,CHECK-NOINDICATOR
+; RUN: opt < %s -asan -asan-module -enable-new-pm=0 -S | FileCheck %s --check-prefixes=CHECK-ALIAS,CHECK-NOINDICATOR
+; RUN: opt < %s -passes='asan-pipeline' -S | FileCheck %s --check-prefixes=CHECK-ALIAS,CHECK-NOINDICATOR
 ; RUN: opt < %s -asan -asan-module -enable-new-pm=0 -asan-use-private-alias=1 -S | FileCheck %s --check-prefixes=CHECK-ALIAS,CHECK-NOINDICATOR
-; RUN: opt < %s -passes='asan-pipeline' -asan-use-private-alias=1 -S | FileCheck %s --check-prefixes=CHECK-ALIAS,CHECK-NOINDICATOR
-; RUN: opt < %s -asan -asan-module -enable-new-pm=0 -asan-use-odr-indicator=1 -S | FileCheck %s --check-prefixes=CHECK-INDICATOR,CHECK-NOALIAS
-; RUN: opt < %s -passes='asan-pipeline' -asan-use-odr-indicator=1 -S | FileCheck %s --check-prefixes=CHECK-INDICATOR,CHECK-NOALIAS
+; RUN: opt < %s -passes='asan-pipeline' -asan-use-private-alias=0 -S | FileCheck %s --check-prefixes=CHECK-NOALIAS,CHECK-NOINDICATOR
+; RUN: opt < %s -asan -asan-module -enable-new-pm=0 -asan-use-odr-indicator=1 -S | FileCheck %s --check-prefixes=CHECK-INDICATOR,CHECK-ALIAS
+; RUN: opt < %s -passes='asan-pipeline' -asan-use-odr-indicator=1 -S | FileCheck %s --check-prefixes=CHECK-INDICATOR,CHECK-ALIAS
 ; RUN: opt < %s -asan -asan-module -enable-new-pm=0 -asan-use-private-alias=1 -asan-use-odr-indicator=1 -S | FileCheck %s --check-prefixes=CHECK-ALIAS,CHECK-INDICATOR
-; RUN: opt < %s -passes='asan-pipeline' -asan-use-private-alias=1 -asan-use-odr-indicator=1 -S | FileCheck %s --check-prefixes=CHECK-ALIAS,CHECK-INDICATOR
+; RUN: opt < %s -passes='asan-pipeline' -asan-use-private-alias=0 -asan-use-odr-indicator=1 -S | FileCheck %s --check-prefixes=CHECK-NOALIAS,CHECK-INDICATOR
 
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
Index: llvm/test/Instrumentation/AddressSanitizer/global_metadata_darwin.ll
===
--- llvm/test/Instrumentation/AddressSanitizer/global_metadata_darwin.ll
+++ llvm/test/Instrumentation/AddressSanitizer/global_metadata_darwin.ll
@@ -2,8 +2,8 @@
 ; allowing dead stripping to be performed, and that the appropriate runtime
 ; routines are invoked.
 
-; RUN: opt < %s -asan -asan-module -enable-new-pm=0 -asan-globals-live-support=1 -S | FileCheck %s
-; RUN: opt < %s -passes='asan-pipeline' -asan-globals-live-support=1 -S | FileCheck %s
+; RUN: opt < %s -asan -asan-module -enable-new-pm=0 -asan-use-private-alias=0 -asan-globals-live-support=1 -S | FileCheck %s
+; RUN: opt < %s -passes='asan-pipeline' -asan-use-private-alias=0 -asan-globals-live-support=1 -S | FileCheck %s
 
 target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-apple-macosx10.11.0"
Index: llvm/test/Instrumentation/AddressSanitizer/global_metadata.ll
===
--- llvm/test/Instrumentation/AddressSanitizer/global_metadata.ll
+++ 

[PATCH] D87974: [Builtin] Add __builtin_zero_non_value_bits.

[Zoe Carver via Phabricator via cfe-commits]

zoecarver added a subscriber: BillyONeal.
zoecarver added a comment.

So, it looks like GCC already uses `__builtin_clear_padding` and MSVC already 
uses `__builtin_zero_non_value_bits`. This patch (obviously) is currently 
implementing `__builtin_zero_non_value_bits ` but, I had planned to update it 
to use `__builtin_clear_padding`. Maybe that's not the best course of action, 
though.

We should all try to agree on _one_ name. CC @BillyONeal @jwakely thoughts?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87974

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

[PATCH] D90893: Support: Change InMemoryFileSystem::addFileNoOwn to take a MemoryBufferRef, NFC

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGe763e032f8bb: Support: Change 
InMemoryFileSystem::addFileNoOwn to take a MemoryBufferRef, NFC (authored by 
dexonsmith).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90893

Files:
  clang/tools/clang-format/ClangFormat.cpp
  llvm/include/llvm/Support/VirtualFileSystem.h
  llvm/lib/Support/VirtualFileSystem.cpp
  llvm/unittests/Support/VirtualFileSystemTest.cpp


Index: llvm/unittests/Support/VirtualFileSystemTest.cpp
===
--- llvm/unittests/Support/VirtualFileSystemTest.cpp
+++ llvm/unittests/Support/VirtualFileSystemTest.cpp
@@ -939,7 +939,7 @@
 
 TEST_F(InMemoryFileSystemTest, OverlayFileNoOwn) {
   auto Buf = MemoryBuffer::getMemBuffer("a");
-  FS.addFileNoOwn("/a", 0, Buf.get());
+  FS.addFileNoOwn("/a", 0, *Buf);
   auto Stat = FS.status("/a");
   ASSERT_FALSE(Stat.getError()) << Stat.getError() << "\n" << FS.toString();
   ASSERT_EQ("/a", Stat->getName());
Index: llvm/lib/Support/VirtualFileSystem.cpp
===
--- llvm/lib/Support/VirtualFileSystem.cpp
+++ llvm/lib/Support/VirtualFileSystem.cpp
@@ -792,14 +792,12 @@
 }
 
 bool InMemoryFileSystem::addFileNoOwn(const Twine , time_t ModificationTime,
-  llvm::MemoryBuffer *Buffer,
+  const llvm::MemoryBufferRef ,
   Optional User,
   Optional Group,
   Optional Type,
   Optional Perms) {
-  return addFile(P, ModificationTime,
- llvm::MemoryBuffer::getMemBuffer(
- Buffer->getBuffer(), Buffer->getBufferIdentifier()),
+  return addFile(P, ModificationTime, llvm::MemoryBuffer::getMemBuffer(Buffer),
  std::move(User), std::move(Group), std::move(Type),
  std::move(Perms));
 }
Index: llvm/include/llvm/Support/VirtualFileSystem.h
===
--- llvm/include/llvm/Support/VirtualFileSystem.h
+++ llvm/include/llvm/Support/VirtualFileSystem.h
@@ -37,6 +37,7 @@
 namespace llvm {
 
 class MemoryBuffer;
+class MemoryBufferRef;
 class Twine;
 
 namespace vfs {
@@ -463,7 +464,8 @@
   /// false if the file or directory already exists in the file system with
   /// different contents.
   bool addFileNoOwn(const Twine , time_t ModificationTime,
-llvm::MemoryBuffer *Buffer, Optional User = None,
+const llvm::MemoryBufferRef ,
+Optional User = None,
 Optional Group = None,
 Optional Type = None,
 Optional Perms = None);
Index: clang/tools/clang-format/ClangFormat.cpp
===
--- clang/tools/clang-format/ClangFormat.cpp
+++ clang/tools/clang-format/ClangFormat.cpp
@@ -174,7 +174,7 @@
 namespace clang {
 namespace format {
 
-static FileID createInMemoryFile(StringRef FileName, MemoryBuffer *Source,
+static FileID createInMemoryFile(StringRef FileName, MemoryBufferRef Source,
  SourceManager , FileManager ,
  llvm::vfs::InMemoryFileSystem *MemFS) {
   MemFS->addFileNoOwn(FileName, 0, Source);
@@ -201,7 +201,7 @@
   IntrusiveRefCntPtr(new DiagnosticIDs),
   new DiagnosticOptions);
   SourceManager Sources(Diagnostics, Files);
-  FileID ID = createInMemoryFile("", Code, Sources, Files,
+  FileID ID = createInMemoryFile("", *Code, Sources, Files,
  InMemoryFileSystem.get());
   if (!LineRanges.empty()) {
 if (!Offsets.empty() || !Lengths.empty()) {
@@ -427,7 +427,7 @@
 IntrusiveRefCntPtr(new DiagnosticIDs),
 new DiagnosticOptions);
 SourceManager Sources(Diagnostics, Files);
-FileID ID = createInMemoryFile(AssumedFileName, Code.get(), Sources, Files,
+FileID ID = createInMemoryFile(AssumedFileName, *Code, Sources, Files,
InMemoryFileSystem.get());
 Rewriter Rewrite(Sources, LangOptions());
 tooling::applyAllReplacements(Replaces, Rewrite);


Index: llvm/unittests/Support/VirtualFileSystemTest.cpp
===
--- llvm/unittests/Support/VirtualFileSystemTest.cpp
+++ llvm/unittests/Support/VirtualFileSystemTest.cpp
@@ -939,7 +939,7 @@
 
 TEST_F(InMemoryFileSystemTest, OverlayFileNoOwn) {
   auto Buf = MemoryBuffer::getMemBuffer("a");
-  FS.addFileNoOwn("/a", 0, Buf.get());
+  FS.addFileNoOwn("/a", 0, *Buf);
   auto Stat = FS.status("/a");
   ASSERT_FALSE(Stat.getError()) 

[clang] e763e03 - Support: Change InMemoryFileSystem::addFileNoOwn to take a MemoryBufferRef, NFC

[Duncan P. N. Exon Smith via cfe-commits]

Author: Duncan P. N. Exon Smith
Date: 2020-12-03T18:09:52-08:00
New Revision: e763e032f8bbf5a4da60d099b1df4cd16e44e139

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

LOG: Support: Change InMemoryFileSystem::addFileNoOwn to take a 
MemoryBufferRef, NFC

Found this by chance when looking at the InMemoryFileSystem API, seems
like an easy cleanup.

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

Added: 


Modified: 
clang/tools/clang-format/ClangFormat.cpp
llvm/include/llvm/Support/VirtualFileSystem.h
llvm/lib/Support/VirtualFileSystem.cpp
llvm/unittests/Support/VirtualFileSystemTest.cpp

Removed: 




diff  --git a/clang/tools/clang-format/ClangFormat.cpp 
b/clang/tools/clang-format/ClangFormat.cpp
index 3a7247deab46..121c9626b261 100644
--- a/clang/tools/clang-format/ClangFormat.cpp
+++ b/clang/tools/clang-format/ClangFormat.cpp
@@ -174,7 +174,7 @@ static cl::list FileNames(cl::Positional, 
cl::desc("[ ...]"),
 namespace clang {
 namespace format {
 
-static FileID createInMemoryFile(StringRef FileName, MemoryBuffer *Source,
+static FileID createInMemoryFile(StringRef FileName, MemoryBufferRef Source,
  SourceManager , FileManager ,
  llvm::vfs::InMemoryFileSystem *MemFS) {
   MemFS->addFileNoOwn(FileName, 0, Source);
@@ -201,7 +201,7 @@ static bool fillRanges(MemoryBuffer *Code,
   IntrusiveRefCntPtr(new DiagnosticIDs),
   new DiagnosticOptions);
   SourceManager Sources(Diagnostics, Files);
-  FileID ID = createInMemoryFile("", Code, Sources, Files,
+  FileID ID = createInMemoryFile("", *Code, Sources, Files,
  InMemoryFileSystem.get());
   if (!LineRanges.empty()) {
 if (!Offsets.empty() || !Lengths.empty()) {
@@ -427,7 +427,7 @@ static bool format(StringRef FileName) {
 IntrusiveRefCntPtr(new DiagnosticIDs),
 new DiagnosticOptions);
 SourceManager Sources(Diagnostics, Files);
-FileID ID = createInMemoryFile(AssumedFileName, Code.get(), Sources, Files,
+FileID ID = createInMemoryFile(AssumedFileName, *Code, Sources, Files,
InMemoryFileSystem.get());
 Rewriter Rewrite(Sources, LangOptions());
 tooling::applyAllReplacements(Replaces, Rewrite);

diff  --git a/llvm/include/llvm/Support/VirtualFileSystem.h 
b/llvm/include/llvm/Support/VirtualFileSystem.h
index 055c0e5dd86f..714bd7ed3050 100644
--- a/llvm/include/llvm/Support/VirtualFileSystem.h
+++ b/llvm/include/llvm/Support/VirtualFileSystem.h
@@ -37,6 +37,7 @@
 namespace llvm {
 
 class MemoryBuffer;
+class MemoryBufferRef;
 class Twine;
 
 namespace vfs {
@@ -463,7 +464,8 @@ class InMemoryFileSystem : public FileSystem {
   /// false if the file or directory already exists in the file system with
   /// 
diff erent contents.
   bool addFileNoOwn(const Twine , time_t ModificationTime,
-llvm::MemoryBuffer *Buffer, Optional User = None,
+const llvm::MemoryBufferRef ,
+Optional User = None,
 Optional Group = None,
 Optional Type = None,
 Optional Perms = None);

diff  --git a/llvm/lib/Support/VirtualFileSystem.cpp 
b/llvm/lib/Support/VirtualFileSystem.cpp
index bbde44c30caa..697383d55d88 100644
--- a/llvm/lib/Support/VirtualFileSystem.cpp
+++ b/llvm/lib/Support/VirtualFileSystem.cpp
@@ -792,14 +792,12 @@ bool InMemoryFileSystem::addFile(const Twine , time_t 
ModificationTime,
 }
 
 bool InMemoryFileSystem::addFileNoOwn(const Twine , time_t ModificationTime,
-  llvm::MemoryBuffer *Buffer,
+  const llvm::MemoryBufferRef ,
   Optional User,
   Optional Group,
   Optional Type,
   Optional Perms) {
-  return addFile(P, ModificationTime,
- llvm::MemoryBuffer::getMemBuffer(
- Buffer->getBuffer(), Buffer->getBufferIdentifier()),
+  return addFile(P, ModificationTime, llvm::MemoryBuffer::getMemBuffer(Buffer),
  std::move(User), std::move(Group), std::move(Type),
  std::move(Perms));
 }

diff  --git a/llvm/unittests/Support/VirtualFileSystemTest.cpp 
b/llvm/unittests/Support/VirtualFileSystemTest.cpp
index 64982b9e2160..4cc209de1a50 100644
--- a/llvm/unittests/Support/VirtualFileSystemTest.cpp
+++ b/llvm/unittests/Support/VirtualFileSystemTest.cpp
@@ -939,7 +939,7 @@ TEST_F(InMemoryFileSystemTest, OverlayFile) {
 
 TEST_F(InMemoryFileSystemTest, OverlayFileNoOwn) {
   auto Buf = MemoryBuffer::getMemBuffer("a");
-  FS.addFileNoOwn("/a", 0, 

[PATCH] D92361: [trivial-abi] Support types without a copy or move constructor.

[Zoe Carver via Phabricator via cfe-commits]

zoecarver added a comment.

@rjmccall Thanks for all that information. You're right; thinking about it in 
the context of four value operations is helpful.

> Hmm. My first instinct is to say that a type that "adds new information" 
> about its special members — i.e. that explicitly declares its destructor or 
> copy/move constructors for some reason other than to immediately explicitly 
> default them — should always be treated conservatively in the absence of a 
> trivial_abi attribute. So a type that explicitly deletes all its copy/move 
> constructors should never be treated as destructively movable without the 
> attribute. A type with implicitly-deleted copy/move constructors due solely 
> to subobjects that are still destructively movable should still be 
> destructively movable, I think.

Just to be clear, this is the opposite of what we were talking about earlier. 
Earlier I was suggesting that a type without the trival_abi attribute (such as 
`S1`), and without any move/copy constructors because of a subobject (`S0`) 
that had the attribute, should //not// be destructively movable. But, I see 
merit in both sides of the argument, so I'm happy to do it either way.

So, if we're all in agreement, I'll update this patch so that `S1` is passed 
directly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92361

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

[PATCH] D92480: [llvm] Unify interface of (ThreadSafe)?RefCountedBase

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

I've committed 2e83ccc2ee89110659f3cb313968a0c970d4 
 which 
takes parts of this patch - specifically fixing the test that was already 
intended to test the RefCountedBase's copy constructor, but didn't (because the 
test called the default constructor instead) and extending it to test/cover (& 
also fix) the similar behavior in ThreadSafeRefCountedBase.

That should leave this proposed patch to add the dtors and assertions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92480

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

[PATCH] D90275: [clang][IR] Add support for leaf attribute

[Gulfem Savrun Yeniceri via Phabricator via cfe-commits]

gulfem marked an inline comment as not done.
gulfem added inline comments.



Comment at: clang/include/clang/Basic/Attr.td:1435
+  let Spellings = [GCC<"leaf">];
+  let Subjects = SubjectList<[Function]>;
+  let Documentation = [Undocumented];

gulfem wrote:
> gulfem wrote:
> > aaron.ballman wrote:
> > > gulfem wrote:
> > > > aaron.ballman wrote:
> > > > > gulfem wrote:
> > > > > > aaron.ballman wrote:
> > > > > > > gulfem wrote:
> > > > > > > > aaron.ballman wrote:
> > > > > > > > > gulfem wrote:
> > > > > > > > > > aaron.ballman wrote:
> > > > > > > > > > > gulfem wrote:
> > > > > > > > > > > > aaron.ballman wrote:
> > > > > > > > > > > > > Should this attribute also be supported on things 
> > > > > > > > > > > > > like ObjC method decls or other function-like 
> > > > > > > > > > > > > interfaces?
> > > > > > > > > > > > Do I need to do anything else to support this attribute 
> > > > > > > > > > > > in Objective-C as well?
> > > > > > > > > > > > I think we should support it in all the C languages 
> > > > > > > > > > > > family.
> > > > > > > > > > > >I think we should support it in all the C languages 
> > > > > > > > > > > >family.
> > > > > > > > > > > 
> > > > > > > > > > > That's already happening automatically -- there's a C and 
> > > > > > > > > > > C++ spelling available for it and the attribute doesn't 
> > > > > > > > > > > specify that it requires a particular language mode or 
> > > > > > > > > > > target.
> > > > > > > > > > > 
> > > > > > > > > > > > Do I need to do anything else to support this attribute 
> > > > > > > > > > > > in Objective-C as well?
> > > > > > > > > > > You can add multiple subjects to the list here, so you 
> > > > > > > > > > > can have this apply to `Function, ObjCMethod` for both of 
> > > > > > > > > > > those. Another one to consider is whether this attribute 
> > > > > > > > > > > can be written on a block declaration (like a lambda, but 
> > > > > > > > > > > with different syntax). Beyond that, it's mostly just 
> > > > > > > > > > > documentation, devising the test cases to ensure the ObjC 
> > > > > > > > > > > functionality behaves as expected, possibly some codegen 
> > > > > > > > > > > changes, etc.
> > > > > > > > > > AFAIK, users can specify function attributes in lambda 
> > > > > > > > > > expressions.
> > > > > > > > > > Lambda functions can only be accessed/called by the 
> > > > > > > > > > functions in the same translation unit, right?
> > > > > > > > > > Leaf attribute does not have any effect on the functions 
> > > > > > > > > > that are defined in the same translation unit.
> > > > > > > > > > For this reason, I'm thinking that leaf attribute would not 
> > > > > > > > > > have any effect if they are used in lambda expressions.
> > > > > > > > > > Do you agree with me?
> > > > > > > > > > AFAIK, users can specify function attributes in lambda 
> > > > > > > > > > expressions.
> > > > > > > > > 
> > > > > > > > > I always forget that you can do that for declaration 
> > > > > > > > > attributes using GNU-style syntax...
> > > > > > > > > 
> > > > > > > > > > Lambda functions can only be accessed/called by the 
> > > > > > > > > > functions in the same translation unit, right?
> > > > > > > > > 
> > > > > > > > > Not necessarily, you could pass one across TU boundaries like 
> > > > > > > > > a function pointer, for instance. e.g.,
> > > > > > > > > ```
> > > > > > > > > // TU1.cpp
> > > > > > > > > void foo() {
> > > > > > > > >   auto l = []() { ... };
> > > > > > > > >   bar(l);
> > > > > > > > > }
> > > > > > > > > 
> > > > > > > > > // TU2.cpp
> > > > > > > > > void bar(auto func) {
> > > > > > > > >   func();
> > > > > > > > > }
> > > > > > > > > ```
> > > > > > > > > Not necessarily, you could pass one across TU boundaries like 
> > > > > > > > > a function pointer, for instance. e.g.,
> > > > > > > > As I mentioned before, leaf attribute is specifically intended 
> > > > > > > > for library functions and I think all the existing usage of 
> > > > > > > > leaf attribute is in the library function declarations. For 
> > > > > > > > this reason, I think we do not need to support them for 
> > > > > > > > lambdas. Is that reasonable?
> > > > > > > > 
> > > > > > > > For this reason, I think we do not need to support them for 
> > > > > > > > lambdas. Is that reasonable?
> > > > > > > 
> > > > > > > Is this considered a library function?
> > > > > > > 
> > > > > > > ```
> > > > > > > struct S {
> > > > > > >   void f(); // Is this a library function?
> > > > > > >   void operator()(); // How about this?
> > > > > > > };
> > > > > > > ```
> > > > > > > If the answer is "no", then I think we only need to support 
> > > > > > > `FunctionDecl` and nothing else (not even `ObjCMethodDecl`, which 
> > > > > > > is like a member function for ObjC). If the answer is "yes", then 
> > > > > > > it's not clear to me whether lambdas should or should not be 
> > > > > > > supported given that the attribute on the 

[PATCH] D92361: [trivial-abi] Support types without a copy or move constructor.

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In D92361#2432578 , @ahatanak wrote:

> It seems like you are discussing the case where a class/struct annotated with 
> `trivial_abi` contains a member that isn't destructively movable. In that 
> case, clang correctly diagnoses it today. For example, if you remove the 
> attribute from `S2` in the above example and add it to `S3` instead, it warns 
> that `trivial_abi` cannot be applied to `S3` because `S2` is a non-trivial 
> class type.
>
> What I wasn't sure was whether `S1` (which isn't annotated with `trivial_abi` 
> in the original code I posted) should be treated as a destructively movable 
> type despite having all its copy/move constructors deleted when its only 
> member (`s0`) is destructively movable.
>
> Based on the discussion we had a few years ago 
> (http://lists.llvm.org/pipermail/cfe-dev/2017-November/055966.html), I think 
> the answer is yes, but I just wanted to confirm.

Hmm.  My first instinct is to say that a type that "adds new information" about 
its special members — i.e. that explicitly declares its destructor or copy/move 
constructors for some reason other than to immediately explicitly default them 
— should always be treated conservatively in the absence of a `trivial_abi` 
attribute.  So a type that explicitly deletes all its copy/move constructors 
should never be treated as destructively movable without the attribute.  A type 
with implicitly-deleted copy/move constructors due solely to subobjects that 
are still destructively movable should still be destructively movable, I think.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92361

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

[PATCH] D92078: [asan] Default to -asan-use-private-alias=1

[Vitaly Buka via Phabricator via cfe-commits]

vitalybuka added a comment.

alias change is LGTM, we can try to figure-out how to roll out 
-fsanitize-address-use-odr-indicator




Comment at: compiler-rt/test/asan/TestCases/Linux/odr-vtable.cpp:5
+// RUN: %clangxx_asan -fno-rtti -DBUILD_SO1 -fPIC -shared -mllvm 
-asan-use-private-alias=0 %s -o %dynamiclib1
+// RUN: %clangxx_asan -fno-rtti -DBUILD_SO2 -fPIC -shared -mllvm 
-asan-use-private-alias=0 %s -o %dynamiclib2
 // RUN: %clangxx_asan -fno-rtti %s %ld_flags_rpath_exe1 %ld_flags_rpath_exe2 
-o %t

now we don't test default behavior
can you add CHECKs for alias=1 as well?
same below


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92078

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

[PATCH] D87216: [NewPM] Support --print-before/after in NPM

[Arthur Eubanks via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG2f0de582949d: [NewPM] Support --print-before/after in NPM 
(authored by aeubanks).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87216

Files:
  llvm/include/llvm/IR/IRPrintingPasses.h
  llvm/include/llvm/IR/PassInstrumentation.h
  llvm/include/llvm/IR/PrintPasses.h
  llvm/include/llvm/Passes/StandardInstrumentations.h
  llvm/lib/Analysis/CallGraphSCCPass.cpp
  llvm/lib/Analysis/LoopInfo.cpp
  llvm/lib/Analysis/LoopPass.cpp
  llvm/lib/CodeGen/MachineFunctionPrinterPass.cpp
  llvm/lib/IR/CMakeLists.txt
  llvm/lib/IR/IRPrintingPasses.cpp
  llvm/lib/IR/LegacyPassManager.cpp
  llvm/lib/IR/PassInstrumentation.cpp
  llvm/lib/IR/PrintPasses.cpp
  llvm/lib/Passes/PassBuilder.cpp
  llvm/lib/Passes/StandardInstrumentations.cpp
  llvm/test/CodeGen/Generic/print-after.ll
  llvm/test/Other/loop-pass-printer.ll
  llvm/test/Other/print-before-after.ll
  llvm/utils/gn/secondary/llvm/lib/IR/BUILD.gn

Index: llvm/utils/gn/secondary/llvm/lib/IR/BUILD.gn
===
--- llvm/utils/gn/secondary/llvm/lib/IR/BUILD.gn
+++ llvm/utils/gn/secondary/llvm/lib/IR/BUILD.gn
@@ -59,6 +59,7 @@
 "PassManager.cpp",
 "PassRegistry.cpp",
 "PassTimingInfo.cpp",
+"PrintPasses.cpp",
 "ProfileSummary.cpp",
 "SafepointIRVerifier.cpp",
 "Statepoint.cpp",
Index: llvm/test/Other/print-before-after.ll
===
--- /dev/null
+++ llvm/test/Other/print-before-after.ll
@@ -0,0 +1,33 @@
+; RUN: not --crash opt < %s -disable-output -passes='no-op-module' -print-before=bleh 2>&1 | FileCheck %s --check-prefix=NONE --allow-empty
+; RUN: not --crash opt < %s -disable-output -passes='no-op-module' -print-after=bleh 2>&1 | FileCheck %s --check-prefix=NONE --allow-empty
+; RUN: opt < %s -disable-output -passes='no-op-module' -print-before=no-op-function 2>&1 | FileCheck %s --check-prefix=NONE --allow-empty
+; RUN: opt < %s -disable-output -passes='no-op-module' -print-after=no-op-function 2>&1 | FileCheck %s --check-prefix=NONE --allow-empty
+; RUN: opt < %s -disable-output -passes='no-op-module,no-op-function' -print-before=no-op-module 2>&1 | FileCheck %s --check-prefix=ONCE
+; RUN: opt < %s -disable-output -passes='no-op-module,no-op-function' -print-after=no-op-module 2>&1 | FileCheck %s --check-prefix=ONCE
+; RUN: opt < %s -disable-output -passes='no-op-function' -print-before=no-op-function 2>&1 | FileCheck %s --check-prefix=ONCE
+; RUN: opt < %s -disable-output -passes='no-op-function' -print-after=no-op-function 2>&1 | FileCheck %s --check-prefix=ONCE
+; RUN: opt < %s -disable-output -passes='no-op-module,no-op-function' -print-before=no-op-function --print-module-scope 2>&1 | FileCheck %s --check-prefix=TWICE
+; RUN: opt < %s -disable-output -passes='no-op-module,no-op-function' -print-after=no-op-function --print-module-scope 2>&1 | FileCheck %s --check-prefix=TWICE
+
+; NONE-NOT: @foo
+; NONE-NOT: @bar
+
+; ONCE: @foo
+; ONCE: @bar
+; ONCE-NOT: @foo
+; ONCE-NOT: @bar
+
+; TWICE: @foo
+; TWICE: @bar
+; TWICE: @foo
+; TWICE: @bar
+; TWICE-NOT: @foo
+; TWICE-NOT: @bar
+
+define void @foo() {
+  ret void
+}
+
+define void @bar() {
+  ret void
+}
Index: llvm/test/Other/loop-pass-printer.ll
===
--- llvm/test/Other/loop-pass-printer.ll
+++ llvm/test/Other/loop-pass-printer.ll
@@ -1,23 +1,23 @@
 ; This test checks -print-after/before on loop passes
 ; Besides of the loop itself it should be dumping loop pre-header and exits.
 ;
-; RUN: opt -enable-new-pm=0 < %s 2>&1 -disable-output \
+; RUN: opt < %s 2>&1 -disable-output \
 ; RUN: 	   -loop-deletion -print-before=loop-deletion \
 ; RUN:	   | FileCheck %s -check-prefix=DEL
 ; RUN: opt < %s 2>&1 -disable-output \
-; RUN: 	   -passes='loop(loop-deletion)' -print-before-all \
+; RUN: 	   -passes='loop(loop-deletion)' -print-before=loop-deletion \
 ; RUN:	   | FileCheck %s -check-prefix=DEL
 ; RUN: opt -enable-new-pm=0 < %s 2>&1 -disable-output \
 ; RUN: 	   -loop-unroll -print-after=loop-unroll -filter-print-funcs=bar \
 ; RUN:	   | FileCheck %s -check-prefix=BAR -check-prefix=BAR-OLD
 ; RUN: opt < %s 2>&1 -disable-output \
-; RUN: 	   -passes='require,loop(loop-unroll-full)' -print-after-all -filter-print-funcs=bar \
+; RUN: 	   -passes='require,loop(loop-unroll-full)' -print-after=loop-unroll-full -filter-print-funcs=bar \
 ; RUN:	   | FileCheck %s -check-prefix=BAR
 ; RUN: opt -enable-new-pm=0 < %s 2>&1 -disable-output \
 ; RUN: 	   -loop-unroll -print-after=loop-unroll -filter-print-funcs=foo -print-module-scope \
 ; RUN:	   | FileCheck %s -check-prefix=FOO-MODULE -check-prefix=FOO-MODULE-OLD
 ; RUN: opt < %s 2>&1 -disable-output \
-; RUN: 	   

[PATCH] D92600: [ASTImporter] Add support for importing GenericSelectionExpr AST nodes.

[Tom Roeder via Phabricator via cfe-commits]

tmroeder updated this revision to Diff 309406.
tmroeder added a comment.

Renamed context to ToCtx as suggested.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92600

Files:
  clang/docs/LibASTMatchersReference.html
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/lib/AST/ASTImporter.cpp
  clang/lib/ASTMatchers/ASTMatchersInternal.cpp
  clang/lib/ASTMatchers/Dynamic/Registry.cpp
  clang/lib/Analysis/ExprMutationAnalyzer.cpp
  clang/test/ASTMerge/generic-selection-expr/Inputs/generic.c
  clang/test/ASTMerge/generic-selection-expr/test.c
  clang/unittests/AST/ASTImporterTest.cpp
  clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp

Index: clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
@@ -985,6 +985,11 @@
   EXPECT_TRUE(matches("int* i = __null;", gnuNullExpr()));
 }
 
+TEST_P(ASTMatchersTest, GenericSelectionExpr) {
+  EXPECT_TRUE(matches("void f() { (void)_Generic(1, int: 1, float: 2.0); }",
+  genericSelectionExpr()));
+}
+
 TEST_P(ASTMatchersTest, AtomicExpr) {
   EXPECT_TRUE(matches("void foo() { int *ptr; __atomic_load_n(ptr, 1); }",
   atomicExpr()));
Index: clang/unittests/AST/ASTImporterTest.cpp
===
--- clang/unittests/AST/ASTImporterTest.cpp
+++ clang/unittests/AST/ASTImporterTest.cpp
@@ -279,6 +279,15 @@
  functionDecl(hasDescendant(gnuNullExpr(hasType(isInteger());
 }
 
+TEST_P(ImportExpr, ImportGenericSelectionExpr) {
+  MatchVerifier Verifier;
+
+  testImport(
+  "void declToImport() { int x; (void)_Generic(x, int: 0, float: 1); }",
+  Lang_C99, "", Lang_C99, Verifier,
+  functionDecl(hasDescendant(genericSelectionExpr(;
+}
+
 TEST_P(ImportExpr, ImportCXXNullPtrLiteralExpr) {
   MatchVerifier Verifier;
   testImport(
@@ -1088,6 +1097,36 @@
 ToChooseExpr->isConditionDependent());
 }
 
+TEST_P(ASTImporterOptionSpecificTestBase, ImportGenericSelectionExpr) {
+  Decl *From, *To;
+  std::tie(From, To) = getImportedDecl(
+  R"(
+  int declToImport() {
+int x;
+return _Generic(x, int: 0, default: 1);
+  }
+  )",
+  Lang_C99, "", Lang_C99);
+
+  auto ToResults =
+  match(genericSelectionExpr().bind("expr"), To->getASTContext());
+  auto FromResults =
+  match(genericSelectionExpr().bind("expr"), From->getASTContext());
+
+  const GenericSelectionExpr *FromGenericSelectionExpr =
+  selectFirst("expr", FromResults);
+  ASSERT_TRUE(FromGenericSelectionExpr);
+
+  const GenericSelectionExpr *ToGenericSelectionExpr =
+  selectFirst("expr", ToResults);
+  ASSERT_TRUE(ToGenericSelectionExpr);
+
+  EXPECT_EQ(FromGenericSelectionExpr->isResultDependent(),
+ToGenericSelectionExpr->isResultDependent());
+  EXPECT_EQ(FromGenericSelectionExpr->getResultIndex(),
+ToGenericSelectionExpr->getResultIndex());
+}
+
 TEST_P(ASTImporterOptionSpecificTestBase,
ImportFunctionWithBackReferringParameter) {
   Decl *From, *To;
Index: clang/test/ASTMerge/generic-selection-expr/test.c
===
--- /dev/null
+++ clang/test/ASTMerge/generic-selection-expr/test.c
@@ -0,0 +1,4 @@
+// RUN: %clang_cc1 -std=c11 -emit-pch -o %t.ast %S/Inputs/generic.c
+// RUN: %clang_cc1 -std=c11 -ast-merge %t.ast -fsyntax-only -verify %s
+// expected-no-diagnostics
+
Index: clang/test/ASTMerge/generic-selection-expr/Inputs/generic.c
===
--- /dev/null
+++ clang/test/ASTMerge/generic-selection-expr/Inputs/generic.c
@@ -0,0 +1,7 @@
+void f() {
+  int x;
+  float y;
+  _Static_assert(_Generic(x, float: 0, int: 1), "Incorrect semantics of _Generic");
+  _Static_assert(_Generic(y, float: 1, int: 0), "Incorrect semantics of _Generic");
+}
+
Index: clang/lib/Analysis/ExprMutationAnalyzer.cpp
===
--- clang/lib/Analysis/ExprMutationAnalyzer.cpp
+++ clang/lib/Analysis/ExprMutationAnalyzer.cpp
@@ -95,10 +95,6 @@
   return Node.isPotentiallyEvaluated();
 }
 
-const ast_matchers::internal::VariadicDynCastAllOfMatcher
-genericSelectionExpr;
-
 AST_MATCHER_P(GenericSelectionExpr, hasControllingExpr,
   ast_matchers::internal::Matcher, InnerMatcher) {
   return InnerMatcher.matches(*Node.getControllingExpr(), Finder, Builder);
Index: clang/lib/ASTMatchers/Dynamic/Registry.cpp
===
--- clang/lib/ASTMatchers/Dynamic/Registry.cpp
+++ clang/lib/ASTMatchers/Dynamic/Registry.cpp
@@ -241,6 +241,7 @@
   REGISTER_MATCHER(functionProtoType);
   REGISTER_MATCHER(functionTemplateDecl);
   

[PATCH] D92617: [DWARF] Allow toolchain to adjust specified DWARF version.

[David Blaikie via Phabricator via cfe-commits]

dblaikie added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3957
+  RenderDebugEnablingArgs(Args, CmdArgs, DebugInfoKind,
+  std::min(DWARFVersion, TC.getMaxDwarfVersion()),
   DebuggerTuning);

I think, ideally, the DWARFVersion calculation would happen up `if (EmitDwarf) 
{` block where all default and explicit dwarf version calculations are done.

I guess it's not done that way because of the gembed-source error path? 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92617

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

[PATCH] D92617: [DWARF] Allow toolchain to adjust specified DWARF version.

[Artem Belevich via Phabricator via cfe-commits]

tra added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3857
 
+  auto AdjustedDwarfVersion = TC.GetAdjustedDwarfVersion(DWARFVersion);
+

dblaikie wrote:
> I worry a bit about having both DWARFVersion and AdjustedDwarfVersion in this 
> scope (I'd worry about some codepaths using one when they needed to use the 
> other, etc) - if at all possible, would be good to just override DWARFVersion 
> with the adjusted value. And probably could/should be up in the "EmitDwarf" 
> block above that's handling default dwarf versions and overrides of it?
Ive updated the implementation to make it less ambiguous. PTAL.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92617

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

[PATCH] D92617: [DWARF] Allow toolchain to adjust specified DWARF version.

[Artem Belevich via Phabricator via cfe-commits]

tra updated this revision to Diff 309404.
tra edited the summary of this revision.
tra added a comment.

Updated according to Devid's feedback.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92617

Files:
  clang/include/clang/Driver/ToolChain.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/Cuda.h
  clang/test/Driver/cuda-unsupported-debug-options.cu


Index: clang/test/Driver/cuda-unsupported-debug-options.cu
===
--- clang/test/Driver/cuda-unsupported-debug-options.cu
+++ clang/test/Driver/cuda-unsupported-debug-options.cu
@@ -18,5 +18,7 @@
 // CHECK-NOT: debug information option '{{.*}}' is not supported for target 
'x86
 // CHECK: "-triple" "nvptx64-nvidia-cuda"
 // CHECK-NOT: 
{{-compress-debug|-fdebug-info-for-profiling|lldb|codeview|module-format|embed-source|debug-info-macro|gnu-pubnames|generate-arange-section|generate-type-units}}
+// Make sure we do not see any dwarf version other than 2, regardless of 
what's used on the host side.
+// CHECK-NOT: {{-dwarf-version=[^2]}}
 // CHECK: "-triple" "x86_64
 // CHECK-SAME: 
{{-compress-debug|-fdebug-info-for-profiling|split-dwarf|lldb|codeview|module-format|embed-source|debug-info-macro|gnu-pubnames|generate-arange-section|generate-type-units}}
Index: clang/lib/Driver/ToolChains/Cuda.h
===
--- clang/lib/Driver/ToolChains/Cuda.h
+++ clang/lib/Driver/ToolChains/Cuda.h
@@ -185,6 +185,8 @@
  const llvm::opt::ArgList ) const override;
 
   unsigned GetDefaultDwarfVersion() const override { return 2; }
+  // NVPTX supports only DWARF2.
+  unsigned getMaxDwarfVersion() const override { return 2; }
 
   const ToolChain 
   CudaInstallationDetector CudaInstallation;
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -3918,8 +3918,15 @@
 if (DWARFVersion < 5)
   D.Diag(diag::err_drv_argument_only_allowed_with)
   << A->getAsString(Args) << "-gdwarf-5";
-else if (checkDebugInfoOption(A, Args, D, TC))
-  CmdArgs.push_back("-gembed-source");
+else {
+  if (TC.getMaxDwarfVersion() < 5)
+// The toolchain has reduced allowed dwarf version, so we can't enable
+// -gembed-source.
+D.Diag(diag::warn_drv_unsupported_debug_info_opt_for_target)
+<< A->getAsString(Args) << TC.getTripleString();
+  else if (checkDebugInfoOption(A, Args, D, TC))
+CmdArgs.push_back("-gembed-source");
+}
   }
 
   if (EmitCodeView) {
@@ -3946,7 +3953,8 @@
   DebugInfoKind <= codegenoptions::DebugDirectivesOnly)
 DebugInfoKind = codegenoptions::DebugLineTablesOnly;
 
-  RenderDebugEnablingArgs(Args, CmdArgs, DebugInfoKind, DWARFVersion,
+  RenderDebugEnablingArgs(Args, CmdArgs, DebugInfoKind,
+  std::min(DWARFVersion, TC.getMaxDwarfVersion()),
   DebuggerTuning);
 
   // -fdebug-macro turns on macro debug info generation.
Index: clang/include/clang/Driver/ToolChain.h
===
--- clang/include/clang/Driver/ToolChain.h
+++ clang/include/clang/Driver/ToolChain.h
@@ -27,6 +27,7 @@
 #include "llvm/Support/VersionTuple.h"
 #include "llvm/Target/TargetOptions.h"
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -489,6 +490,11 @@
   // to the contrary.
   virtual unsigned GetDefaultDwarfVersion() const { return 4; }
 
+  // Some toolchains may have different restrictions on the DWARF version and
+  // may need to adjust it. E.g. NVPTX may need to enforce DWARF2 even when 
host
+  // compilation uses DWARF5.
+  virtual unsigned getMaxDwarfVersion() const { return UINT_MAX; }
+
   // True if the driver should assume "-fstandalone-debug"
   // in the absence of an option specifying otherwise,
   // provided that debugging was requested in the first place.


Index: clang/test/Driver/cuda-unsupported-debug-options.cu
===
--- clang/test/Driver/cuda-unsupported-debug-options.cu
+++ clang/test/Driver/cuda-unsupported-debug-options.cu
@@ -18,5 +18,7 @@
 // CHECK-NOT: debug information option '{{.*}}' is not supported for target 'x86
 // CHECK: "-triple" "nvptx64-nvidia-cuda"
 // CHECK-NOT: {{-compress-debug|-fdebug-info-for-profiling|lldb|codeview|module-format|embed-source|debug-info-macro|gnu-pubnames|generate-arange-section|generate-type-units}}
+// Make sure we do not see any dwarf version other than 2, regardless of what's used on the host side.
+// CHECK-NOT: {{-dwarf-version=[^2]}}
 // CHECK: "-triple" "x86_64
 // CHECK-SAME: 

[PATCH] D92361: [trivial-abi] Support types without a copy or move constructor.

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added a comment.

It seems like you are discussing the case where a class/struct annotated with 
`trivial_abi` contains a member that isn't destructively movable. In that case, 
clang correctly diagnoses it today. For example, if you remove the attribute 
from `S2` in the above example and add it to `S3` instead, it warns that 
`trivial_abi` cannot be applied to `S3` because `S2` is a non-trivial class 
type.

What I wasn't sure was whether `S1` (which isn't annotated with `trivial_abi` 
in the original code I posted) should be treated as a destructively movable 
type despite having all its copy/move constructors deleted when its only member 
(`s0`) is destructively movable.

Based on the discussion we had a few years ago 
(http://lists.llvm.org/pipermail/cfe-dev/2017-November/055966.html), I think 
the answer is yes, but I just wanted to confirm.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92361

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

[PATCH] D92617: [DWARF] Allow toolchain to adjust specified DWARF version.

[David Blaikie via Phabricator via cfe-commits]

dblaikie added inline comments.



Comment at: clang/include/clang/Driver/ToolChain.h:495
+  // compilation uses DWARF5.
+  virtual unsigned GetAdjustedDwarfVersion(unsigned v) const { return v; }
+

Given these semantics (pass/return the version) maybe "AdjustDwarfVersion" or 
the like might be more suitable (are there other similar functions in this API 
That can/do this sort of pass/return API?).

Alternatively, a narrower/simpler API could be "GetMaxDwarfVersion", say, which 
just returns int_max by default and 2 for NVPTX?



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3857
 
+  auto AdjustedDwarfVersion = TC.GetAdjustedDwarfVersion(DWARFVersion);
+

I worry a bit about having both DWARFVersion and AdjustedDwarfVersion in this 
scope (I'd worry about some codepaths using one when they needed to use the 
other, etc) - if at all possible, would be good to just override DWARFVersion 
with the adjusted value. And probably could/should be up in the "EmitDwarf" 
block above that's handling default dwarf versions and overrides of it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92617

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

[PATCH] D90888: Frontend: Remove redundant call to CompilerInstance::setFileManager, NFC

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG99b823c2eba3: Frontend: Remove redundant call to 
CompilerInstance::setFileManager, NFC (authored by dexonsmith).
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90888

Files:
  clang/lib/Frontend/ASTUnit.cpp


Index: clang/lib/Frontend/ASTUnit.cpp
===
--- clang/lib/Frontend/ASTUnit.cpp
+++ clang/lib/Frontend/ASTUnit.cpp
@@ -1186,9 +1186,6 @@
 TopLevelDeclsInPreamble.clear();
   }
 
-  // Create a file manager object to provide access to and cache the 
filesystem.
-  Clang->setFileManager(());
-
   // Create the source manager.
   Clang->setSourceManager(());
 


Index: clang/lib/Frontend/ASTUnit.cpp
===
--- clang/lib/Frontend/ASTUnit.cpp
+++ clang/lib/Frontend/ASTUnit.cpp
@@ -1186,9 +1186,6 @@
 TopLevelDeclsInPreamble.clear();
   }
 
-  // Create a file manager object to provide access to and cache the filesystem.
-  Clang->setFileManager(());
-
   // Create the source manager.
   Clang->setSourceManager(());
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 99b823c - Frontend: Remove redundant call to CompilerInstance::setFileManager, NFC

[Duncan P. N. Exon Smith via cfe-commits]

Author: Duncan P. N. Exon Smith
Date: 2020-12-03T16:10:59-08:00
New Revision: 99b823c2eba391877a0fcd6bc5f03f0d9f0077cb

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

LOG: Frontend: Remove redundant call to CompilerInstance::setFileManager, NFC

`ASTUnit::Parse` sets up the `FileManager` earlier in the same function,
ensuring `ASTUnit::getFileManager()` matches `Clang->getFileManager()`.
Remove the later call to `setFileManager(getFileManager())` since it
does nothing.

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

Added: 


Modified: 
clang/lib/Frontend/ASTUnit.cpp

Removed: 




diff  --git a/clang/lib/Frontend/ASTUnit.cpp b/clang/lib/Frontend/ASTUnit.cpp
index c2aba4102361..c8ac7eaa9ab8 100644
--- a/clang/lib/Frontend/ASTUnit.cpp
+++ b/clang/lib/Frontend/ASTUnit.cpp
@@ -1186,9 +1186,6 @@ bool 
ASTUnit::Parse(std::shared_ptr PCHContainerOps,
 TopLevelDeclsInPreamble.clear();
   }
 
-  // Create a file manager object to provide access to and cache the 
filesystem.
-  Clang->setFileManager(());
-
   // Create the source manager.
   Clang->setSourceManager(());
 



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

[PATCH] D92617: [DWARF] Allow toolchain to adjust specified DWARF version.

[Artem Belevich via Phabricator via cfe-commits]

tra created this revision.
tra added reviewers: dblaikie, ABataev.
Herald added a subscriber: bixia.
tra requested review of this revision.
Herald added a project: clang.

This is needed for CUDA compilation where NVPTX back-end only supports DWARF2,
but host compilation should be allowed to use newer DWARF versions.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D92617

Files:
  clang/include/clang/Driver/ToolChain.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/Cuda.h
  clang/test/Driver/cuda-unsupported-debug-options.cu


Index: clang/test/Driver/cuda-unsupported-debug-options.cu
===
--- clang/test/Driver/cuda-unsupported-debug-options.cu
+++ clang/test/Driver/cuda-unsupported-debug-options.cu
@@ -18,5 +18,7 @@
 // CHECK-NOT: debug information option '{{.*}}' is not supported for target 
'x86
 // CHECK: "-triple" "nvptx64-nvidia-cuda"
 // CHECK-NOT: 
{{-compress-debug|-fdebug-info-for-profiling|lldb|codeview|module-format|embed-source|debug-info-macro|gnu-pubnames|generate-arange-section|generate-type-units}}
+// Make sure we do not see any dwarf version other than 2, regardless of 
what's used on the host side.
+// CHECK-NOT: {{-dwarf-version=[^2]}}
 // CHECK: "-triple" "x86_64
 // CHECK-SAME: 
{{-compress-debug|-fdebug-info-for-profiling|split-dwarf|lldb|codeview|module-format|embed-source|debug-info-macro|gnu-pubnames|generate-arange-section|generate-type-units}}
Index: clang/lib/Driver/ToolChains/Cuda.h
===
--- clang/lib/Driver/ToolChains/Cuda.h
+++ clang/lib/Driver/ToolChains/Cuda.h
@@ -185,6 +185,8 @@
  const llvm::opt::ArgList ) const override;
 
   unsigned GetDefaultDwarfVersion() const override { return 2; }
+  // NVPTX supports only DWARF2.
+  unsigned GetAdjustedDwarfVersion(unsigned v) const override { return 2; }
 
   const ToolChain 
   CudaInstallationDetector CudaInstallation;
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -3854,6 +3854,8 @@
 DWARFVersion = ExplicitVersion;
   }
 
+  auto AdjustedDwarfVersion = TC.GetAdjustedDwarfVersion(DWARFVersion);
+
   // -gline-directives-only supported only for the DWARF debug info.
   if (DWARFVersion == 0 && DebugInfoKind == 
codegenoptions::DebugDirectivesOnly)
 DebugInfoKind = codegenoptions::NoDebugInfo;
@@ -3918,6 +3920,11 @@
 if (DWARFVersion < 5)
   D.Diag(diag::err_drv_argument_only_allowed_with)
   << A->getAsString(Args) << "-gdwarf-5";
+else if (AdjustedDwarfVersion < 5)
+  // The toolchain has reduced allowed dwarf version, so we can't enable
+  // -gembed-source.
+  D.Diag(diag::warn_drv_unsupported_debug_info_opt_for_target)
+  << A->getAsString(Args) << TC.getTripleString();
 else if (checkDebugInfoOption(A, Args, D, TC))
   CmdArgs.push_back("-gembed-source");
   }
@@ -3946,7 +3953,7 @@
   DebugInfoKind <= codegenoptions::DebugDirectivesOnly)
 DebugInfoKind = codegenoptions::DebugLineTablesOnly;
 
-  RenderDebugEnablingArgs(Args, CmdArgs, DebugInfoKind, DWARFVersion,
+  RenderDebugEnablingArgs(Args, CmdArgs, DebugInfoKind, AdjustedDwarfVersion,
   DebuggerTuning);
 
   // -fdebug-macro turns on macro debug info generation.
Index: clang/include/clang/Driver/ToolChain.h
===
--- clang/include/clang/Driver/ToolChain.h
+++ clang/include/clang/Driver/ToolChain.h
@@ -489,6 +489,11 @@
   // to the contrary.
   virtual unsigned GetDefaultDwarfVersion() const { return 4; }
 
+  // Some toolchains may have different restrictions on the DWARF version and
+  // may need to adjust it. E.g. NVPTX may need to enforce DWARF2 even when 
host
+  // compilation uses DWARF5.
+  virtual unsigned GetAdjustedDwarfVersion(unsigned v) const { return v; }
+
   // True if the driver should assume "-fstandalone-debug"
   // in the absence of an option specifying otherwise,
   // provided that debugging was requested in the first place.


Index: clang/test/Driver/cuda-unsupported-debug-options.cu
===
--- clang/test/Driver/cuda-unsupported-debug-options.cu
+++ clang/test/Driver/cuda-unsupported-debug-options.cu
@@ -18,5 +18,7 @@
 // CHECK-NOT: debug information option '{{.*}}' is not supported for target 'x86
 // CHECK: "-triple" "nvptx64-nvidia-cuda"
 // CHECK-NOT: {{-compress-debug|-fdebug-info-for-profiling|lldb|codeview|module-format|embed-source|debug-info-macro|gnu-pubnames|generate-arange-section|generate-type-units}}
+// Make sure we do not see any dwarf version other than 2, regardless of what's used on the host side.
+// CHECK-NOT: {{-dwarf-version=[^2]}}
 // CHECK: "-triple" "x86_64
 // 

[clang] eccc734 - P0857R0: Parse a requires-clause after an explicit

[Richard Smith via cfe-commits]

Author: Richard Smith
Date: 2020-12-03T15:54:16-08:00
New Revision: eccc734a69c0c012ae3160887b65a535b35ead3e

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

LOG: P0857R0: Parse a requires-clause after an explicit
template-parameter-list in a lambda.

This implements one of the missing parts of P0857R0. Mark it as not done
on the cxx_status page given that it's still incomplete.

Added: 


Modified: 
clang/include/clang/Sema/ScopeInfo.h
clang/include/clang/Sema/Sema.h
clang/lib/Parse/ParseExprCXX.cpp
clang/lib/Sema/SemaLambda.cpp
clang/test/SemaTemplate/concepts.cpp
clang/www/cxx_status.html

Removed: 




diff  --git a/clang/include/clang/Sema/ScopeInfo.h 
b/clang/include/clang/Sema/ScopeInfo.h
index ff2a20923415..8ec74cafeeca 100644
--- a/clang/include/clang/Sema/ScopeInfo.h
+++ b/clang/include/clang/Sema/ScopeInfo.h
@@ -849,6 +849,11 @@ class LambdaScopeInfo final :
   /// Source range covering the explicit template parameter list (if it 
exists).
   SourceRange ExplicitTemplateParamsRange;
 
+  /// The requires-clause immediately following the explicit template parameter
+  /// list, if any. (Note that there may be another requires-clause included as
+  /// part of the lambda-declarator.)
+  ExprResult RequiresClause;
+
   /// If this is a generic lambda, and the template parameter
   /// list has been created (from the TemplateParams) then store
   /// a reference to it (cache it to avoid reconstructing it).

diff  --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index fc19e04e6c5d..b3ede504542f 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -6551,7 +6551,8 @@ class Sema final {
   /// on a lambda (if it exists) in C++2a.
   void ActOnLambdaExplicitTemplateParameterList(SourceLocation LAngleLoc,
 ArrayRef TParams,
-SourceLocation RAngleLoc);
+SourceLocation RAngleLoc,
+ExprResult RequiresClause);
 
   /// Introduce the lambda parameters into scope.
   void addLambdaParameters(

diff  --git a/clang/lib/Parse/ParseExprCXX.cpp 
b/clang/lib/Parse/ParseExprCXX.cpp
index 96df2c932bea..4b5703d79f28 100644
--- a/clang/lib/Parse/ParseExprCXX.cpp
+++ b/clang/lib/Parse/ParseExprCXX.cpp
@@ -1268,7 +1268,6 @@ ExprResult Parser::ParseLambdaExpressionAfterIntroducer(
   << A.getAttrName()->getName();
   };
 
-  // FIXME: Consider allowing this as an extension for GCC compatibiblity.
   MultiParseScope TemplateParamScope(*this);
   if (Tok.is(tok::less)) {
 Diag(Tok, getLangOpts().CPlusPlus20
@@ -1288,8 +1287,17 @@ ExprResult Parser::ParseLambdaExpressionAfterIntroducer(
   Diag(RAngleLoc,
diag::err_lambda_template_parameter_list_empty);
 } else {
+  ExprResult RequiresClause;
+  if (TryConsumeToken(tok::kw_requires)) {
+RequiresClause =
+Actions.ActOnRequiresClause(ParseConstraintLogicalOrExpression(
+/*IsTrailingRequiresClause=*/false));
+if (RequiresClause.isInvalid())
+  SkipUntil({tok::l_brace, tok::l_paren}, StopAtSemi | 
StopBeforeMatch);
+  }
+
   Actions.ActOnLambdaExplicitTemplateParameterList(
-  LAngleLoc, TemplateParams, RAngleLoc);
+  LAngleLoc, TemplateParams, RAngleLoc, RequiresClause);
   ++CurTemplateDepthTracker;
 }
   }

diff  --git a/clang/lib/Sema/SemaLambda.cpp b/clang/lib/Sema/SemaLambda.cpp
index c2eb663147c2..ec1db50cfaaa 100644
--- a/clang/lib/Sema/SemaLambda.cpp
+++ b/clang/lib/Sema/SemaLambda.cpp
@@ -233,7 +233,7 @@ getGenericLambdaTemplateParameterList(LambdaScopeInfo *LSI, 
Sema ) {
 /*L angle loc*/ LSI->ExplicitTemplateParamsRange.getBegin(),
 LSI->TemplateParams,
 /*R angle loc*/LSI->ExplicitTemplateParamsRange.getEnd(),
-nullptr);
+LSI->RequiresClause.get());
   }
   return LSI->GLTemplateParameterList;
 }
@@ -520,7 +520,8 @@ void Sema::finishLambdaExplicitCaptures(LambdaScopeInfo 
*LSI) {
 
 void Sema::ActOnLambdaExplicitTemplateParameterList(SourceLocation LAngleLoc,
 ArrayRef 
TParams,
-SourceLocation RAngleLoc) {
+SourceLocation RAngleLoc,
+ExprResult RequiresClause) 
{
   LambdaScopeInfo *LSI = getCurLambda();
   assert(LSI && "Expected a lambda scope");
   assert(LSI->NumExplicitTemplateParams == 0 &&
@@ -533,6 +534,7 @@ void 
Sema::ActOnLambdaExplicitTemplateParameterList(SourceLocation LAngleLoc,
   

[clang] be162f4 - PR45699: Fix crash if an unexpanded parameter pack appears in a

[Richard Smith via cfe-commits]

Author: Richard Smith
Date: 2020-12-03T15:26:06-08:00
New Revision: be162f4c0e8563c8de510121435281ae628c8654

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

LOG: PR45699: Fix crash if an unexpanded parameter pack appears in a
requires-clause.

Added: 


Modified: 
clang/include/clang/AST/ExprCXX.h
clang/include/clang/AST/RecursiveASTVisitor.h
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/include/clang/Sema/Sema.h
clang/lib/AST/ExprCXX.cpp
clang/lib/Parse/ParseTemplate.cpp
clang/lib/Sema/SemaDeclCXX.cpp
clang/test/SemaTemplate/concepts.cpp

Removed: 




diff  --git a/clang/include/clang/AST/ExprCXX.h 
b/clang/include/clang/AST/ExprCXX.h
index 2f89b43267b6..1efa78fc4294 100644
--- a/clang/include/clang/AST/ExprCXX.h
+++ b/clang/include/clang/AST/ExprCXX.h
@@ -2029,6 +2029,9 @@ class LambdaExpr final : public Expr,
   /// invented by use of an auto parameter).
   ArrayRef getExplicitTemplateParameters() const;
 
+  /// Get the trailing requires clause, if any.
+  Expr *getTrailingRequiresClause() const;
+
   /// Whether this is a generic lambda.
   bool isGenericLambda() const { return getTemplateParameterList(); }
 

diff  --git a/clang/include/clang/AST/RecursiveASTVisitor.h 
b/clang/include/clang/AST/RecursiveASTVisitor.h
index 0a36ec9ad687..612e60cf4df1 100644
--- a/clang/include/clang/AST/RecursiveASTVisitor.h
+++ b/clang/include/clang/AST/RecursiveASTVisitor.h
@@ -2503,17 +2503,12 @@ DEF_TRAVERSE_STMT(LambdaExpr, {
 TypeLoc TL = S->getCallOperator()->getTypeSourceInfo()->getTypeLoc();
 FunctionProtoTypeLoc Proto = TL.getAsAdjusted();
 
-for (Decl *D : S->getExplicitTemplateParameters()) {
-  // Visit explicit template parameters.
-  TRY_TO(TraverseDecl(D));
-}
+TRY_TO(TraverseTemplateParameterListHelper(S->getTemplateParameterList()));
 if (S->hasExplicitParameters()) {
   // Visit parameters.
   for (unsigned I = 0, N = Proto.getNumParams(); I != N; ++I)
 TRY_TO(TraverseDecl(Proto.getParam(I)));
 }
-if (S->hasExplicitResultType())
-  TRY_TO(TraverseTypeLoc(Proto.getReturnLoc()));
 
 auto *T = Proto.getTypePtr();
 for (const auto  : T->exceptions())
@@ -2522,6 +2517,10 @@ DEF_TRAVERSE_STMT(LambdaExpr, {
 if (Expr *NE = T->getNoexceptExpr())
   TRY_TO_TRAVERSE_OR_ENQUEUE_STMT(NE);
 
+if (S->hasExplicitResultType())
+  TRY_TO(TraverseTypeLoc(Proto.getReturnLoc()));
+TRY_TO_TRAVERSE_OR_ENQUEUE_STMT(S->getTrailingRequiresClause());
+
 TRY_TO_TRAVERSE_OR_ENQUEUE_STMT(S->getBody());
   }
   ShouldVisitChildren = false;

diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 44195cc9db45..23f374987c92 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -5158,7 +5158,8 @@ def err_unexpanded_parameter_pack : Error<
   "using declaration|friend declaration|qualifier|initializer|default 
argument|"
   "non-type template parameter type|exception type|partial specialization|"
   "__if_exists name|__if_not_exists name|lambda|block|type constraint|"
-  "requirement}0 contains%plural{0: an|:}1 unexpanded parameter pack"
+  "requirement|requires clause}0 "
+  "contains%plural{0: an|:}1 unexpanded parameter pack"
   "%plural{0:|1: %2|2:s %2 and %3|:s %2, %3, ...}1">;
 
 def err_pack_expansion_without_parameter_packs : Error<

diff  --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 8b4a18ab11d6..fc19e04e6c5d 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -2674,6 +2674,7 @@ class Sema final {
 SkipBodyInfo *SkipBody = nullptr);
   void ActOnStartTrailingRequiresClause(Scope *S, Declarator );
   ExprResult ActOnFinishTrailingRequiresClause(ExprResult ConstraintExpr);
+  ExprResult ActOnRequiresClause(ExprResult ConstraintExpr);
   void ActOnStartOfObjCMethodDef(Scope *S, Decl *D);
   bool isObjCMethodDecl(Decl *D) {
 return D && isa(D);
@@ -7927,6 +7928,9 @@ class Sema final {
 
 // A requirement in a requires-expression.
 UPPC_Requirement,
+
+// A requires-clause.
+UPPC_RequiresClause,
   };
 
   /// Diagnose unexpanded parameter packs.

diff  --git a/clang/lib/AST/ExprCXX.cpp b/clang/lib/AST/ExprCXX.cpp
index b7f677051ea2..8dc9d4296e14 100644
--- a/clang/lib/AST/ExprCXX.cpp
+++ b/clang/lib/AST/ExprCXX.cpp
@@ -1271,6 +1271,10 @@ ArrayRef 
LambdaExpr::getExplicitTemplateParameters() const {
   return Record->getLambdaExplicitTemplateParameters();
 }
 
+Expr *LambdaExpr::getTrailingRequiresClause() const {
+  return getCallOperator()->getTrailingRequiresClause();
+}
+
 bool LambdaExpr::isMutable() const { return 

[PATCH] D91944: OpenMP 5.0 metadirective

[Alok Mishra via Phabricator via cfe-commits]

alokmishra.besu added inline comments.



Comment at: clang/include/clang/Serialization/ASTBitCodes.h:1952
-  STMT_MS_DEPENDENT_EXISTS,   // MSDependentExistsStmt
-  EXPR_LAMBDA,// LambdaExpr
   STMT_COROUTINE_BODY,

jdoerfert wrote:
> alokmishra.besu wrote:
> > jdoerfert wrote:
> > > Unrelated.
> > Only STMT_OMP_META_DIRECTIVE was added. Rest was formatted by git 
> > clang-format
> > Rest was formatted by git clang-format
> 
> Please don't format unrelated code. 
Removed.



Comment at: clang/lib/Parse/ParseOpenMP.cpp:2264
+  Actions.StartOpenMPDSABlock(DirKind, DirName, Actions.getCurScope(),
+  Loc);
+  int paren = 0;

jdoerfert wrote:
> alokmishra.besu wrote:
> > jdoerfert wrote:
> > > Should we not go back to the original code handling "directives" instead? 
> > > This looks like it is copied here.
> > Unfortunately we cannot go to the original code handling since the original 
> > code handling assumes that the directive always ends with 
> > annot_pragma_openmp_end, while here it will always end with ')'.
> > In specification 5.0, since we are choosing only 1 directive, the body of 
> > the while block remains the same as the original code. Only the condition 
> > of the while block changes. In specification 5.1, we will need to generate 
> > code for dynamic handling and even the body will differ as we might need to 
> > generate AST node for multiple directives. It is best if we handle this 
> > code here for easier handling of 5.1 code, than in the original code space.
> > I will add a TODO comment here.
> > Unfortunately we cannot go to the original code handling since the original 
> > code handling assumes that the directive always ends with 
> > annot_pragma_openmp_end, while here it will always end with ')'.
> 
> Let's add a flag to the original handling to make this possible then. Copying 
> it is going to create more long term problems.
Done.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91944

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

[PATCH] D91944: OpenMP 5.0 metadirective

[Alok Mishra via Phabricator via cfe-commits]

alokmishra.besu updated this revision to Diff 309382.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91944

Files:
  clang/include/clang-c/Index.h
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/include/clang/AST/StmtOpenMP.h
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Basic/StmtNodes.td
  clang/include/clang/Sema/Sema.h
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/lib/AST/OpenMPClause.cpp
  clang/lib/AST/StmtOpenMP.cpp
  clang/lib/AST/StmtPrinter.cpp
  clang/lib/AST/StmtProfile.cpp
  clang/lib/Basic/OpenMPKinds.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/Parse/ParseOpenMP.cpp
  clang/lib/Sema/SemaExceptionSpec.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTReaderStmt.cpp
  clang/lib/Serialization/ASTWriterStmt.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
  clang/test/OpenMP/metadirective_ast_print.c
  clang/test/OpenMP/metadirective_ast_print.cpp
  clang/test/OpenMP/metadirective_device_kind_codegen.c
  clang/test/OpenMP/metadirective_device_kind_codegen.cpp
  clang/test/OpenMP/metadirective_empty.cpp
  clang/test/OpenMP/metadirective_implementation_codegen.c
  clang/test/OpenMP/metadirective_implementation_codegen.cpp
  clang/test/OpenMP/metadirective_messages.cpp
  clang/tools/libclang/CIndex.cpp
  clang/tools/libclang/CXCursor.cpp
  llvm/include/llvm/Frontend/OpenMP/OMP.td
  llvm/include/llvm/Frontend/OpenMP/OMPKinds.def

Index: llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
===
--- llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
+++ llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
@@ -115,6 +115,7 @@
 __OMP_CLAUSE_NO_CLASS(uniform)
 __OMP_CLAUSE_NO_CLASS(device_type)
 __OMP_CLAUSE_NO_CLASS(match)
+__OMP_CLAUSE_NO_CLASS(when)
 
 __OMP_IMPLICIT_CLAUSE_CLASS(depobj, "depobj", OMPDepobjClause)
 __OMP_IMPLICIT_CLAUSE_CLASS(flush, "flush", OMPFlushClause)
Index: llvm/include/llvm/Frontend/OpenMP/OMP.td
===
--- llvm/include/llvm/Frontend/OpenMP/OMP.td
+++ llvm/include/llvm/Frontend/OpenMP/OMP.td
@@ -59,6 +59,7 @@
   let clangClass = "OMPCollapseClause";
   let flangClassValue = "ScalarIntConstantExpr";
 }
+def OMPC_When: Clause<"when"> {}
 def OMPC_Default : Clause<"default"> {
   let clangClass = "OMPDefaultClause";
   let flangClass = "OmpDefaultClause";
@@ -295,6 +296,14 @@
 // Definition of OpenMP directives
 //===--===//
 
+def OMP_Metadirective : Directive<"metadirective"> {
+  let allowedClauses = [
+VersionedClause
+  ];
+  let allowedOnceClauses = [
+VersionedClause
+  ];
+}
 def OMP_ThreadPrivate : Directive<"threadprivate"> {}
 def OMP_Parallel : Directive<"parallel"> {
   let allowedClauses = [
Index: clang/tools/libclang/CXCursor.cpp
===
--- clang/tools/libclang/CXCursor.cpp
+++ clang/tools/libclang/CXCursor.cpp
@@ -639,6 +639,9 @@
   case Stmt::MSDependentExistsStmtClass:
 K = CXCursor_UnexposedStmt;
 break;
+  case Stmt::OMPMetaDirectiveClass:
+K = CXCursor_OMPMetaDirective;
+break;
   case Stmt::OMPParallelDirectiveClass:
 K = CXCursor_OMPParallelDirective;
 break;
Index: clang/tools/libclang/CIndex.cpp
===
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -5525,6 +5525,8 @@
 return cxstring::createRef("CXXAccessSpecifier");
   case CXCursor_ModuleImportDecl:
 return cxstring::createRef("ModuleImport");
+  case CXCursor_OMPMetaDirective:
+return cxstring::createRef("OMPMetaDirective");
   case CXCursor_OMPParallelDirective:
 return cxstring::createRef("OMPParallelDirective");
   case CXCursor_OMPSimdDirective:
Index: clang/test/OpenMP/metadirective_messages.cpp
===
--- /dev/null
+++ clang/test/OpenMP/metadirective_messages.cpp
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -verify -fopenmp -x c++ -std=c++14 -emit-llvm %s
+
+void foo() {
+#pragma omp metadirective // expected-error {{expected expression}}
+  ;
+#pragma omp metadirective when() // expected-error {{expected valid context selector in when clause}} expected-error {{expected expression}} expected-warning {{expected identifier or string literal describing a context set; set skipped}} expected-note {{context set options are: 'construct' 'device' 'implementation' 'user'}} expected-note {{the ignored set spans until here}}
+  ;
+#pragma omp metadirective when(device{}) // expected-error {{expected valid context selector in when clause}} 

[PATCH] D91980: [OpenMP] Add initial support for `omp [begin/end] assumes`

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert updated this revision to Diff 309381.
jdoerfert added a comment.

Early exit


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91980

Files:
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Parse/ParseOpenMP.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaLambda.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/assumes_codegen.cpp
  clang/test/OpenMP/assumes_include_nvptx.cpp
  clang/test/OpenMP/assumes_messages.c
  clang/test/OpenMP/assumes_print.cpp
  clang/test/OpenMP/assumes_template_print.cpp
  llvm/include/llvm/Frontend/OpenMP/OMP.td
  llvm/include/llvm/Frontend/OpenMP/OMPConstants.h
  llvm/include/llvm/Frontend/OpenMP/OMPKinds.def

Index: llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
===
--- llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
+++ llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
@@ -1224,3 +1224,27 @@
 #undef __OMP_REQUIRES_TRAIT
 #undef OMP_REQUIRES_TRAIT
 ///}
+
+
+/// Assumption clauses
+///
+///{
+
+#ifdef OMP_ASSUME_CLAUSE
+#define __OMP_ASSUME_CLAUSE(Identifier, StartsWith, HasDirectiveList, HasExpression) \
+OMP_ASSUME_CLAUSE(Identifier, StartsWith, HasDirectiveList, HasExpression)
+#else
+#define __OMP_ASSUME_CLAUSE(...)
+#endif
+
+__OMP_ASSUME_CLAUSE("ext_", true, false, false)
+__OMP_ASSUME_CLAUSE("absent", false, true, false)
+__OMP_ASSUME_CLAUSE("contains", false, true, false)
+__OMP_ASSUME_CLAUSE("holds", false, false, true)
+__OMP_ASSUME_CLAUSE("no_openmp", false, false, false)
+__OMP_ASSUME_CLAUSE("no_openmp_routines", false, false, false)
+__OMP_ASSUME_CLAUSE("no_parallelism", false, false, false)
+
+#undef __OMP_ASSUME_CLAUSE
+#undef OMP_ASSUME_CLAUSE
+///}
Index: llvm/include/llvm/Frontend/OpenMP/OMPConstants.h
===
--- llvm/include/llvm/Frontend/OpenMP/OMPConstants.h
+++ llvm/include/llvm/Frontend/OpenMP/OMPConstants.h
@@ -16,6 +16,7 @@
 
 #include "llvm/ADT/BitmaskEnum.h"
 
+#include "llvm/ADT/StringRef.h"
 #include "llvm/Frontend/OpenMP/OMP.h.inc"
 
 namespace llvm {
@@ -79,6 +80,33 @@
 #define OMP_IDENT_FLAG(Enum, ...) constexpr auto Enum = omp::IdentFlag::Enum;
 #include "llvm/Frontend/OpenMP/OMPKinds.def"
 
+/// Helper to describe assume clauses.
+struct AssumptionClauseMappingInfo {
+  /// The identifier describing the (beginning of the) clause.
+  llvm::StringLiteral Identifier;
+  /// Flag to determine if the identifier is a full name or the start of a name.
+  bool StartsWith;
+  /// Flag to determine if a directive lists follows.
+  bool HasDirectiveList;
+  /// Flag to determine if an expression follows.
+  bool HasExpression;
+};
+
+/// All known assume clauses.
+static constexpr AssumptionClauseMappingInfo AssumptionClauseMappings[] = {
+#define OMP_ASSUME_CLAUSE(Identifier, StartsWith, HasDirectiveList,\
+  HasExpression)   \
+  {Identifier, StartsWith, HasDirectiveList, HasExpression},
+#include "llvm/Frontend/OpenMP/OMPKinds.def"
+};
+
+inline std::string getAllAssumeClauseOptions() {
+  std::string S;
+  for (const AssumptionClauseMappingInfo  : AssumptionClauseMappings)
+S += (S.empty() ? "'" : "', '") + ACMI.Identifier.str();
+  return S + "'";
+}
+
 } // end namespace omp
 
 } // end namespace llvm
Index: llvm/include/llvm/Frontend/OpenMP/OMP.td
===
--- llvm/include/llvm/Frontend/OpenMP/OMP.td
+++ llvm/include/llvm/Frontend/OpenMP/OMP.td
@@ -1590,6 +1590,9 @@
 VersionedClause
   ];
 }
+def OMP_Assumes : Directive<"assumes"> {}
+def OMP_BeginAssumes : Directive<"begin assumes"> {}
+def OMP_EndAssumes : Directive<"end assumes"> {}
 def OMP_BeginDeclareVariant : Directive<"begin declare variant"> {}
 def OMP_EndDeclareVariant : Directive<"end declare variant"> {}
 def OMP_ParallelWorkshare : Directive<"parallel workshare"> {
Index: clang/test/OpenMP/assumes_template_print.cpp
===
--- /dev/null
+++ clang/test/OpenMP/assumes_template_print.cpp
@@ -0,0 +1,91 @@
+// RUN: %clang_cc1 -verify -fopenmp -ast-print %s | FileCheck %s
+// RUN: %clang_cc1 -fopenmp -x c++ -std=c++11 -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp -std=c++11 -include-pch %t -fsyntax-only -verify %s -ast-print | FileCheck %s
+
+// RUN: %clang_cc1 -verify -fopenmp-simd -ast-print %s | FileCheck %s
+// RUN: %clang_cc1 -fopenmp-simd -x c++ -std=c++11 -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp-simd -std=c++11 -include-pch %t -fsyntax-only -verify %s -ast-print | FileCheck %s
+// expected-no-diagnostics
+
+// It is unclear if we want to annotate the template instantiations, e.g., S::foo, or not in the two
+// situations shown below. Since it is always fair to drop 

[PATCH] D92130: [HIP] cmath promote long double args to double

[Aaron Enye Shi via Phabricator via cfe-commits]

ashi1 added a comment.

In D92130#2423801 , @tra wrote:

> LGTM in general. Will defer to Sam as it's HIP.
>
> Nit:
>
>> long double, promote them to double, and use the fp64
>
> I'd say `long double` -> `double` qualifies as a demotion.

Thanks, I've changed the commit description to demote.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92130

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

[PATCH] D92130: [HIP] cmath promote long double args to double

[Aaron Enye Shi via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGba2612ce01ea: [HIP] cmath demote long double args to double 
(authored by ashi1).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92130

Files:
  clang/lib/Headers/__clang_hip_cmath.h


Index: clang/lib/Headers/__clang_hip_cmath.h
===
--- clang/lib/Headers/__clang_hip_cmath.h
+++ clang/lib/Headers/__clang_hip_cmath.h
@@ -224,6 +224,8 @@
   static double __test(long long);
   static double __test(unsigned long long);
   static double __test(double);
+  // No support for long double, use double instead.
+  static double __test(long double);
 
   typedef decltype(__test(std::declval<_Tp>())) type;
   static const bool value = !std::is_same::value;


Index: clang/lib/Headers/__clang_hip_cmath.h
===
--- clang/lib/Headers/__clang_hip_cmath.h
+++ clang/lib/Headers/__clang_hip_cmath.h
@@ -224,6 +224,8 @@
   static double __test(long long);
   static double __test(unsigned long long);
   static double __test(double);
+  // No support for long double, use double instead.
+  static double __test(long double);
 
   typedef decltype(__test(std::declval<_Tp>())) type;
   static const bool value = !std::is_same::value;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] ba2612c - [HIP] cmath demote long double args to double

[Aaron En Ye Shi via cfe-commits]

Author: Aaron En Ye Shi
Date: 2020-12-03T23:00:14Z
New Revision: ba2612ce01eae5859ae7adb99161775fb2c4e0b2

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

LOG: [HIP] cmath demote long double args to double

Since there is no ROCm Device Library support for
long double, demote them to double, and use the fp64
math functions.

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

Added: 


Modified: 
clang/lib/Headers/__clang_hip_cmath.h

Removed: 




diff  --git a/clang/lib/Headers/__clang_hip_cmath.h 
b/clang/lib/Headers/__clang_hip_cmath.h
index 00519a9795bc..3a702587ee17 100644
--- a/clang/lib/Headers/__clang_hip_cmath.h
+++ b/clang/lib/Headers/__clang_hip_cmath.h
@@ -224,6 +224,8 @@ template  struct __numeric_type {
   static double __test(long long);
   static double __test(unsigned long long);
   static double __test(double);
+  // No support for long double, use double instead.
+  static double __test(long double);
 
   typedef decltype(__test(std::declval<_Tp>())) type;
   static const bool value = !std::is_same::value;



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

[PATCH] D91980: [OpenMP] Add initial support for `omp [begin/end] assumes`

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added inline comments.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:3203-3206
+  if (!SkippedClauses && Assumptions.empty())
+Diag(Loc, diag::err_omp_no_clause_for_directive)
+<< llvm::omp::getAllAssumeClauseOptions()
+<< llvm::omp::getOpenMPDirectiveName(DKind);

ABataev wrote:
> Early exit from the function in case of error?
We can't always because we want to create an (empty) scope so we can diagnose 
the `end assumes` properly. I'll check in the non-scope case though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91980

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

[PATCH] D92606: Clean up debug locations for logical-and/or expressions

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

Probably worth separating these changes/fixes/tests (looks like 3 different 
changes?) - at least it'd help me understand what each one of the changes does, 
separately (I'm especially curious about the EmitScalarConversion one and would 
like to better understand why the generic Expr handling for ApplyDebugLocation 
isn't working here, if it can be made more robust and/or if the proposed 
solution could be generalized a bit/moved further up in the stack). And is 
there an existing test file/cases these could be put near?




Comment at: clang/lib/CodeGen/CGExprScalar.cpp:4293
+  {
+// There is no need to emit line number for unconditional branch.
+auto NL = ApplyDebugLocation::CreateEmpty(CGF);

rnk wrote:
> I see this is consistent with LAnd handling above, but is there a principled 
> reason for using an empty location here? If I was stopped on this 
> unconditional branch instruction in the debugger, I would expect to see the 
> location of the logical operator.
> 
> No need to change behavior in this patch, I'm just wondering if we can remove 
> this as a followup.
> 
> I dug into the blame, and the comment about "no need" for a location comes 
> from 2011:
> http://github.com/llvm/llvm-project/commit/4d7612744f080d2c0a8639c1f5e41b691e1bba55
> The reasoning seems to be that it improves the gdb test suite.
I /think/ I remember encountering these issues independently/as well when I was 
working on the gdb test suite maybe a couple of years later. Hmm, guess it 
might've overlapped with this work.

6f2e41e0d4421ab376801bfb88d3f197a8e96994 / r128471 provides maybe a bit more 
context: "Do not line number entry for unconditional branches. Usually, users 
do not want to stop at closing '}'."


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92606

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

[PATCH] D92606: Clean up debug locations for logical-and/or expressions

[Paul Robinson via Phabricator via cfe-commits]

probinson added inline comments.



Comment at: clang/lib/CodeGen/CGExprScalar.cpp:4293
+  {
+// There is no need to emit line number for unconditional branch.
+auto NL = ApplyDebugLocation::CreateEmpty(CGF);

rnk wrote:
> I see this is consistent with LAnd handling above, but is there a principled 
> reason for using an empty location here? If I was stopped on this 
> unconditional branch instruction in the debugger, I would expect to see the 
> location of the logical operator.
> 
> No need to change behavior in this patch, I'm just wondering if we can remove 
> this as a followup.
> 
> I dug into the blame, and the comment about "no need" for a location comes 
> from 2011:
> http://github.com/llvm/llvm-project/commit/4d7612744f080d2c0a8639c1f5e41b691e1bba55
> The reasoning seems to be that it improves the gdb test suite.
I have no insight here.  I'm willing to take it on as a follow-up exercise, 
after the FastISel patch goes back in.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92606

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

[PATCH] D92606: Clean up debug locations for logical-and/or expressions

[Reid Kleckner via Phabricator via cfe-commits]

rnk added inline comments.



Comment at: clang/lib/CodeGen/CGExprScalar.cpp:4293
+  {
+// There is no need to emit line number for unconditional branch.
+auto NL = ApplyDebugLocation::CreateEmpty(CGF);

I see this is consistent with LAnd handling above, but is there a principled 
reason for using an empty location here? If I was stopped on this unconditional 
branch instruction in the debugger, I would expect to see the location of the 
logical operator.

No need to change behavior in this patch, I'm just wondering if we can remove 
this as a followup.

I dug into the blame, and the comment about "no need" for a location comes from 
2011:
http://github.com/llvm/llvm-project/commit/4d7612744f080d2c0a8639c1f5e41b691e1bba55
The reasoning seems to be that it improves the gdb test suite.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92606

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

[PATCH] D91893: [clang-tidy] performance-unnecessary-copy-initialization: Prevent false positives when dependent variable is modified.

[Felix Berger via Phabricator via cfe-commits]

flx updated this revision to Diff 309361.
flx added a comment.

Fixed clang tidy warnings.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91893

Files:
  clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
  clang-tools-extra/clang-tidy/utils/DeclRefExprUtils.cpp
  clang-tools-extra/clang-tidy/utils/Matchers.h
  
clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp
@@ -476,3 +476,17 @@
   auto Copy = Orig.reference();
   Update(Copy);
 }
+
+void negativeCopiedFromReferenceToModifiedVar() {
+  ExpensiveToCopyType Orig;
+  const auto  = Orig;
+  const auto NecessaryCopy = Ref;
+  Orig.nonConstMethod();
+}
+
+void negativeCopiedFromGetterOfReferenceToModifiedVar() {
+  ExpensiveToCopyType Orig;
+  const auto  = Orig.reference();
+  const auto NecessaryCopy = Ref.reference();
+  Orig.nonConstMethod();
+}
Index: clang-tools-extra/clang-tidy/utils/Matchers.h
===
--- clang-tools-extra/clang-tidy/utils/Matchers.h
+++ clang-tools-extra/clang-tidy/utils/Matchers.h
@@ -43,6 +43,12 @@
   return referenceType(pointee(qualType(isConstQualified(;
 }
 
+// Returns QualType matcher for pointers to const.
+AST_MATCHER_FUNCTION(ast_matchers::TypeMatcher, isPointerToConst) {
+  using namespace ast_matchers;
+  return pointerType(pointee(qualType(isConstQualified(;
+}
+
 AST_MATCHER_P(NamedDecl, matchesAnyListedName, std::vector,
   NameList) {
   return llvm::any_of(NameList, [](const std::string ) {
Index: clang-tools-extra/clang-tidy/utils/DeclRefExprUtils.cpp
===
--- clang-tools-extra/clang-tidy/utils/DeclRefExprUtils.cpp
+++ clang-tools-extra/clang-tidy/utils/DeclRefExprUtils.cpp
@@ -58,7 +58,7 @@
   SmallPtrSet DeclRefs;
   extractNodesByIdTo(Matches, "declRef", DeclRefs);
   auto ConstReferenceOrValue =
-  qualType(anyOf(referenceType(pointee(qualType(isConstQualified(,
+  qualType(anyOf(matchers::isReferenceToConst(),
  unless(anyOf(referenceType(), pointerType(),
   substTemplateTypeParmType();
   auto ConstReferenceOrValueOrReplaced = qualType(anyOf(
@@ -71,6 +71,20 @@
   Matches =
   match(findAll(cxxConstructExpr(UsedAsConstRefOrValueArg)), Stmt, Context);
   extractNodesByIdTo(Matches, "declRef", DeclRefs);
+  // References and pointers to const assignments.
+  Matches =
+  match(findAll(declStmt(
+has(varDecl(hasType(qualType(matchers::isReferenceToConst())),
+hasInitializer(ignoringImpCasts(DeclRefToVar)),
+Stmt, Context);
+  extractNodesByIdTo(Matches, "declRef", DeclRefs);
+  Matches =
+  match(findAll(declStmt(has(varDecl(
+hasType(qualType(matchers::isPointerToConst())),
+hasInitializer(ignoringImpCasts(unaryOperator(
+hasOperatorName("&"), hasUnaryOperand(DeclRefToVar,
+Stmt, Context);
+  extractNodesByIdTo(Matches, "declRef", DeclRefs);
   return DeclRefs;
 }
 
Index: clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
===
--- clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
+++ clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
@@ -19,6 +19,14 @@
 namespace performance {
 namespace {
 
+using namespace ::clang::ast_matchers;
+using llvm::StringRef;
+using utils::decl_ref_expr::isOnlyUsedAsConst;
+
+static constexpr StringRef ObjectArgId = "objectArg";
+static constexpr StringRef InitFunctionCallId = "initFunctionCall";
+static constexpr StringRef OldVarDeclId = "oldVarDecl";
+
 void recordFixes(const VarDecl , ASTContext ,
  DiagnosticBuilder ) {
   Diagnostic << utils::fixit::changeVarDeclToReference(Var, Context);
@@ -29,10 +37,90 @@
   }
 }
 
-} // namespace
+AST_MATCHER_FUNCTION(StatementMatcher, isConstRefReturningMethodCall) {
+  // Match method call expressions where the `this` argument is only used as
+  // const, this will be checked in `check()` part. This returned const
+  // reference is highly likely to outlive the local const reference of the
+  // variable being declared. The assumption is that the const reference being
+  // returned either points to a global static variable or to a member of the
+  // called object.
+  return cxxMemberCallExpr(
+  

[PATCH] D92001: [ubsan] Fix crash on __builtin_assume_aligned

[Roman Lebedev via Phabricator via cfe-commits]

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

@orivej please can you look where we create AST nodes for these builtins and 
ensure that the "pointer" argument is actually converted into a pointer 
beforehand?
I'm afraid i can't readily point at the examples, but it should be easy-ish..




Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:2524
   // defined.
-  if (Ty->getPointeeType().isVolatileQualified())
+  if (!Ty->getPointeeType().isNull() && 
Ty->getPointeeType().isVolatileQualified())
 return;

aaron.ballman wrote:
> lebedev.ri wrote:
> > vsk wrote:
> > > Is the pointee type associated with a PointerType QualType ever supposed 
> > > to be null? I wonder why this happens, and whether it can cause problems 
> > > in other places.
> > Basically, we can't just use `getPointeeType()` here, but i'm not sure what 
> > should be used instead.
> I'm not super familiar with `__builtin_assume_aligned` but from the GCC docs, 
> it looks like the code from the test case is valid code (so we don't need to 
> add semantic checking that would ensure we don't reach this code path). 
> However, it seems a bit odd to me that we'd get an array type here as opposed 
> to a decayed type which would actually be a pointer.
> 
> I think the issue is further up the call chain, perhaps. `EmitBuiltinExpr()` 
> gets the argument expression and passes it to `emitAlignmentAssumption()` 
> which pulls the type directly out of the expression. It seems like there's an 
> lvalue to rvalue conversion step missing to adjust the type.
Aha, yes, that does sound like the underying problem.
I was thinking about that beforehand, but wasn't sure.


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

https://reviews.llvm.org/D92001

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

[PATCH] D92606: Clean up debug locations for logical-and/or expressions

[Paul Robinson via Phabricator via cfe-commits]

probinson created this revision.
probinson added reviewers: dblaikie, rnk.
probinson added a project: debug-info.
probinson requested review of this revision.
Herald added a project: clang.

Attaches a more appropriate debug location to the PHIs used for the
short-circuit evaluations, and makes logical-and and logical-or
handling consistent.  Also sets the appropriate debug location for all
scalar conversions, which incidentally does the right thing for
int-to-bool conversions in particular.

While this does tidy up the source locations emitted to IR, it doesn't
appear to make any difference in the final object file for simple
tests.  However, it should permit re-applying D91734 
 and friends to
improve the debug info without exposing the regressions that prompted
the reverts in #615f63e.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D92606

Files:
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/test/CodeGen/debug-info-inline-for.c
  clang/test/CodeGen/debug-info-land-lor.c

Index: clang/test/CodeGen/debug-info-land-lor.c
===
--- /dev/null
+++ clang/test/CodeGen/debug-info-land-lor.c
@@ -0,0 +1,36 @@
+// Check that clang emits reasonable debug info for && and ||.
+// RUN: %clang_cc1 -debug-info-kind=limited -emit-llvm -o - %s | FileCheck %s
+int f1(int a, int b) {
+  return a && b; // line 4
+}
+int f2(int a, int b) {
+  return a || b; // line 7
+}
+
+// Each load/icmp pair should have the location of the operand.
+// Each conditional branch/phi pair should have the location of the operator.
+
+// CHECK-LABEL: @f1
+// CHECK:  load {{.*}} %a.addr, {{.*}} !dbg ![[F1A:[0-9]+]]
+// CHECK-NEXT: icmp {{.*}} !dbg ![[F1A]]
+// CHECK-NEXT: br {{.*}} !dbg ![[F1AND:[0-9]+]]
+// CHECK: land.rhs:
+// CHECK-NEXT: load {{.*}} %b.addr, {{.*}} !dbg ![[F1B:[0-9]+]]
+// CHECK-NEXT: icmp {{.*}} !dbg ![[F1B]]
+// CHECK:  phi {{.*}} !dbg ![[F1AND]]
+
+// CHECK-LABEL: @f2
+// CHECK:  load {{.*}} %a.addr, {{.*}} !dbg ![[F2A:[0-9]+]]
+// CHECK-NEXT: icmp {{.*}} !dbg ![[F2A]]
+// CHECK-NEXT: br {{.*}} !dbg ![[F2OR:[0-9]+]]
+// CHECK: lor.rhs:
+// CHECK-NEXT: load {{.*}} %b.addr, {{.*}} !dbg ![[F2B:[0-9]+]]
+// CHECK-NEXT: icmp {{.*}} !dbg ![[F2B]]
+// CHECK:  phi {{.*}} !dbg ![[F2OR]]
+
+// CHECK-DAG:   ![[F1A]] = !DILocation(line: 4, column: 10,
+// CHECK-DAG:   ![[F1B]] = !DILocation(line: 4, column: 15,
+// CHECK-DAG: ![[F1AND]] = !DILocation(line: 4, column: 12,
+// CHECK-DAG:   ![[F2A]] = !DILocation(line: 7, column: 10,
+// CHECK-DAG:   ![[F2B]] = !DILocation(line: 7, column: 15,
+// CHECK-DAG:  ![[F2OR]] = !DILocation(line: 7, column: 12,
Index: clang/test/CodeGen/debug-info-inline-for.c
===
--- clang/test/CodeGen/debug-info-inline-for.c
+++ /dev/null
@@ -1,13 +0,0 @@
-// RUN: %clang_cc1 -debug-info-kind=limited -emit-llvm -o - %s | FileCheck %s
-// Check that clang emits Debug location in the phi instruction
-
-int func(int n) {
-  int a;
-  for(a = 10; a>0 && n++; a--);
-  return n;
-}
-
-// CHECK: land.end:
-// CHECK-NEXT: {{.*}} = phi i1 {{.*}} !dbg ![[DbgLoc:[0-9]+]]
-
-// CHECK: ![[DbgLoc]] = !DILocation(line: 0
Index: clang/lib/CodeGen/CGExprScalar.cpp
===
--- clang/lib/CodeGen/CGExprScalar.cpp
+++ clang/lib/CodeGen/CGExprScalar.cpp
@@ -1197,6 +1197,8 @@
QualType DstType,
SourceLocation Loc,
ScalarConversionOpts Opts) {
+  ApplyDebugLocation DL(CGF, Loc);
+
   // All conversions involving fixed point types should be handled by the
   // EmitFixedPoint family functions. This is done to prevent bloating up this
   // function more, and although fixed point numbers are represented by
@@ -4187,6 +4189,7 @@
   // setting up the PHI node in the Cont Block for this.
   llvm::PHINode *PN = llvm::PHINode::Create(llvm::Type::getInt1Ty(VMContext), 2,
 "", ContBlock);
+  PN->setDebugLoc(Builder.getCurrentDebugLocation());
   for (llvm::pred_iterator PI = pred_begin(ContBlock), PE = pred_end(ContBlock);
PI != PE; ++PI)
 PN->addIncoming(llvm::ConstantInt::getFalse(VMContext), *PI);
@@ -4209,12 +4212,6 @@
   // Insert an entry into the phi node for the edge with the value of RHSCond.
   PN->addIncoming(RHSCond, RHSBlock);
 
-  // Artificial location to preserve the scope information
-  {
-auto NL = ApplyDebugLocation::CreateArtificial(CGF);
-PN->setDebugLoc(Builder.getCurrentDebugLocation());
-  }
-
   // ZExt result to int.
   return Builder.CreateZExtOrBitCast(PN, ResTy, "land.ext");
 }
@@ -4274,6 +4271,7 @@
   // setting up the PHI node in the Cont Block for this.
   llvm::PHINode *PN = llvm::PHINode::Create(llvm::Type::getInt1Ty(VMContext), 2,

[PATCH] D89909: [SYCL] Implement SYCL address space attributes handling

[Alexey Bader via Phabricator via cfe-commits]

bader added inline comments.



Comment at: clang/lib/CodeGen/CGCall.cpp:4596
+
IRFuncTy->getParamType(FirstIRArg));
+}
+

bader wrote:
> rjmccall wrote:
> > This seems problematic; code like this shouldn't be necessary in every 
> > place that uses a pointer value.  The general rule needs to be that 
> > expression emission produces a value of its expected type, so the places 
> > that emit pointers need to be casting to the generic address space.  We can 
> > avoid that in some special expression emitters, like maybe EmitLValue, as 
> > long as the LValue records what happened and can apply the appropriate 
> > transform later.
> > 
> > Also, in IRGen we work as much as possible with AST address spaces rather 
> > than IR address spaces.  That is, code like this that's checking for 
> > address space mismatches needs to be comparing the source AST address space 
> > with the destination AST address space and calling a method on the 
> > TargetInfo when a mismatch is detected, rather than comparing IR address 
> > spaces and directly building an `addrspacecast`.  The idea is that in 
> > principle a target should be able to have two logical address spaces that 
> > are actually implemented in IR with the same underlying address space, with 
> > some arbitrary transform between them.
> @erichkeane, @asavonic, could you help with addressing this concern, please?
I looked into this and managed to replace almost all CodeGen library changes 
with a couple of hooks for SPIR target. The one change, which is still required 
is located in clang/lib/CodeGen/CGDecl.cpp and I think the assertion fixed by 
this change can be reproduced by compiling C++ for AMDGPU target. The assertion 
happens due to invalid bitcast in constant expression and it's triggered by one 
of the cases in clang/test/CodeGenSYCL/address-spaces.cpp:

```
  const char *str = "Hello, world!";
  i = str[0];
  const char *phi_str = i > 2 ? str : "Another hello world!";
  const char *select_null = i > 2 ? "Yet another Hello world" : nullptr;
  const char *select_str_trivial1 = true ? str : "Another hello world!";

```
I also see that using this approach we get much more addrspacecast 
instructions. I hope it won't be problem, I'm going to apply the same change to 
our fork and run additional tests to make sure there no other problems with 
that patch.

@rjmccall, thanks a lot for valuable feedback!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89909

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

[PATCH] D92600: [ASTImporter] Add support for importing GenericSelectionExpr AST nodes.

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added a comment.

`ASTImporter` changes looks good to me.




Comment at: clang/lib/AST/ASTImporter.cpp:6525
+
+  const ASTContext  = Importer.getToContext();
+  if (E->isResultDependent()) {

nitpick `context` -> `ToCtx` to be consistent with other code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92600

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

[PATCH] D89909: [SYCL] Implement SYCL address space attributes handling

[Alexey Bader via Phabricator via cfe-commits]

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

Apply code review comment from @rjmccall


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89909

Files:
  clang/include/clang/AST/Type.h
  clang/include/clang/Basic/AddressSpaces.h
  clang/include/clang/Sema/ParsedAttr.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/TypePrinter.cpp
  clang/lib/Basic/Targets.cpp
  clang/lib/Basic/Targets/AMDGPU.cpp
  clang/lib/Basic/Targets/NVPTX.h
  clang/lib/Basic/Targets/SPIR.h
  clang/lib/Basic/Targets/TCE.h
  clang/lib/Basic/Targets/X86.h
  clang/lib/CodeGen/CGDecl.cpp
  clang/lib/CodeGen/TargetInfo.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/CodeGenSYCL/address-space-cond-op.cpp
  clang/test/CodeGenSYCL/address-space-of-returns.cpp
  clang/test/CodeGenSYCL/address-space-parameter-conversions.cpp
  clang/test/CodeGenSYCL/address-spaces.cpp
  clang/test/SemaSYCL/address-space-parameter-conversions.cpp
  clang/test/SemaTemplate/address_space-dependent.cpp
  llvm/include/llvm/ADT/Triple.h
  llvm/lib/Support/Triple.cpp

Index: llvm/lib/Support/Triple.cpp
===
--- llvm/lib/Support/Triple.cpp
+++ llvm/lib/Support/Triple.cpp
@@ -244,6 +244,8 @@
   case Musl: return "musl";
   case MuslEABI: return "musleabi";
   case MuslEABIHF: return "musleabihf";
+  case SYCLDevice:
+return "sycldevice";
   case Simulator: return "simulator";
   }
 
@@ -529,26 +531,27 @@
 
 static Triple::EnvironmentType parseEnvironment(StringRef EnvironmentName) {
   return StringSwitch(EnvironmentName)
-.StartsWith("eabihf", Triple::EABIHF)
-.StartsWith("eabi", Triple::EABI)
-.StartsWith("gnuabin32", Triple::GNUABIN32)
-.StartsWith("gnuabi64", Triple::GNUABI64)
-.StartsWith("gnueabihf", Triple::GNUEABIHF)
-.StartsWith("gnueabi", Triple::GNUEABI)
-.StartsWith("gnux32", Triple::GNUX32)
-.StartsWith("code16", Triple::CODE16)
-.StartsWith("gnu", Triple::GNU)
-.StartsWith("android", Triple::Android)
-.StartsWith("musleabihf", Triple::MuslEABIHF)
-.StartsWith("musleabi", Triple::MuslEABI)
-.StartsWith("musl", Triple::Musl)
-.StartsWith("msvc", Triple::MSVC)
-.StartsWith("itanium", Triple::Itanium)
-.StartsWith("cygnus", Triple::Cygnus)
-.StartsWith("coreclr", Triple::CoreCLR)
-.StartsWith("simulator", Triple::Simulator)
-.StartsWith("macabi", Triple::MacABI)
-.Default(Triple::UnknownEnvironment);
+  .StartsWith("eabihf", Triple::EABIHF)
+  .StartsWith("eabi", Triple::EABI)
+  .StartsWith("gnuabin32", Triple::GNUABIN32)
+  .StartsWith("gnuabi64", Triple::GNUABI64)
+  .StartsWith("gnueabihf", Triple::GNUEABIHF)
+  .StartsWith("gnueabi", Triple::GNUEABI)
+  .StartsWith("gnux32", Triple::GNUX32)
+  .StartsWith("code16", Triple::CODE16)
+  .StartsWith("gnu", Triple::GNU)
+  .StartsWith("android", Triple::Android)
+  .StartsWith("musleabihf", Triple::MuslEABIHF)
+  .StartsWith("musleabi", Triple::MuslEABI)
+  .StartsWith("musl", Triple::Musl)
+  .StartsWith("msvc", Triple::MSVC)
+  .StartsWith("itanium", Triple::Itanium)
+  .StartsWith("cygnus", Triple::Cygnus)
+  .StartsWith("coreclr", Triple::CoreCLR)
+  .StartsWith("sycldevice", Triple::SYCLDevice)
+  .StartsWith("simulator", Triple::Simulator)
+  .StartsWith("macabi", Triple::MacABI)
+  .Default(Triple::UnknownEnvironment);
 }
 
 static Triple::ObjectFormatType parseFormat(StringRef EnvironmentName) {
Index: llvm/include/llvm/ADT/Triple.h
===
--- llvm/include/llvm/ADT/Triple.h
+++ llvm/include/llvm/ADT/Triple.h
@@ -217,8 +217,9 @@
 Itanium,
 Cygnus,
 CoreCLR,
+SYCLDevice,
 Simulator, // Simulator variants of other systems, e.g., Apple's iOS
-MacABI, // Mac Catalyst variant of Apple's iOS deployment target.
+MacABI,// Mac Catalyst variant of Apple's iOS deployment target.
 LastEnvironmentType = MacABI
   };
   enum ObjectFormatType {
@@ -492,6 +493,10 @@
isMacCatalystEnvironment()));
   }
 
+  bool isSYCLDeviceEnvironment() const {
+return getEnvironment() == Triple::SYCLDevice;
+  }
+
   bool isOSNetBSD() const {
 return getOS() == Triple::NetBSD;
   }
Index: clang/test/SemaTemplate/address_space-dependent.cpp
===
--- clang/test/SemaTemplate/address_space-dependent.cpp
+++ clang/test/SemaTemplate/address_space-dependent.cpp
@@ -43,7 +43,7 @@
 
 template 
 void tooBig() {
-  __attribute__((address_space(I))) int *bounds; // expected-error {{address space is larger than the maximum supported (8388593)}}
+  __attribute__((address_space(I))) int *bounds; // expected-error {{address space is larger than the maximum supported (8388590)}}
 }
 

[PATCH] D79773: [clang-format] Improve clang-formats handling of concepts

[Michael Schellenberger Costa via Phabricator via cfe-commits]

miscco added a comment.

As someone who has extensivly worked with conscepts I cannot stress how much 
this would improve my live


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

https://reviews.llvm.org/D79773

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

[PATCH] D91980: [OpenMP] Add initial support for `omp [begin/end] assumes`

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:3203-3206
+  if (!SkippedClauses && Assumptions.empty())
+Diag(Loc, diag::err_omp_no_clause_for_directive)
+<< llvm::omp::getAllAssumeClauseOptions()
+<< llvm::omp::getOpenMPDirectiveName(DKind);

Early exit from the function in case of error?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91980

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

[PATCH] D92480: [llvm] Unify interface of (ThreadSafe)?RefCountedBase

[David Blaikie via Phabricator via cfe-commits]

dblaikie added inline comments.



Comment at: clang/lib/ASTMatchers/ASTMatchersInternal.cpp:190-196
+// Use a custom deleter for the TrueMatcherInstance ManagedStatic. This 
prevents
+// an assert firing when the refcount is nonzero while running its destructor.
+struct DynMatcherInterfaceDeleter {
+  static void call(void *Ptr) {
+static_cast(Ptr)->Release();
+  }
+};

I think probably the right solution is to have TrueMatcherImpl's dtor Release 
the same way its ctor Retains. Symmetry is nice/helps avoid splitting this 
quirk into two different places.



Comment at: llvm/include/llvm/ADT/IntrusiveRefCntPtr.h:75-80
+  // Copy and move constructors/assignments are no-ops as the RefCount isn't
+  // dictated by the class directly.
   RefCountedBase(const RefCountedBase &) {}
+  RefCountedBase(RefCountedBase &&) {}
+  RefCountedBase =(const RefCountedBase &) { return *this; }
+  RefCountedBase =(RefCountedBase &&) { return *this; }

dexonsmith wrote:
> dexonsmith wrote:
> > njames93 wrote:
> > > njames93 wrote:
> > > > dexonsmith wrote:
> > > > > dblaikie wrote:
> > > > > > I can't quite understand this comment - perhaps you could try 
> > > > > > rephrasing it?
> > > > > I'm not quite following why you needed to add these special members; 
> > > > > it seems like just the destructor would be enough for the assertion; 
> > > > > but if you do need them, can/should they be `= default`?
> > > > They can't be defaulted as we don't want to copy the RefCount.
> > > I understand it in my head, just can't think of the best way to write it 
> > > down.
> > Maybe something like this would work:
> > ```
> > /// Do not copy or move RefCount when constructing or assigning from
> > /// a derived type. These operations don't imply anything about the
> > /// number of IntrusiveRefCntPtr instances.
> > ```
> > WDYT?
> Ah, I get it, thanks for explaining.
Do we need them at all, though?

The code prior to this patch has a copy ctor that "does the right thing" by 
producing a ref count of 0 for the new copy (same as a newly constructed 
object). No move operations will be provided - nor probably should there be 
any, the copy semantics are cheap/correct & not sure there's a more optimized 
implementation for move.

The copy assignment operaotr currently is probably broken (the implicit copy 
assignment is deprecated in the presence of a user-declared copy ctor - so we 
could probably delete that rather than adding it if it's not used (& at least, 
currently if it is used it'd be pretty broken).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92480

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

[PATCH] D79773: [clang-format] Improve clang-formats handling of concepts

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

Having reviewed the current status I'm going to leave this patch with just 
minor changes from where its been for some time, if others feel it has merits 
in landing as experimental support I'd be happy with that. I would rather build 
from a base than try and keep rebuilding the same tower.

The concepts support in clang-format is currently poor, actually, it's 
downright destructive, I've tried to cover the basic concept usage and I'm 
happy to support this feature against individual bug report where I can.


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

https://reviews.llvm.org/D79773

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

[PATCH] D92597: ARCMigrate: Initialize fields in EditEntry inline, NFC

[Jonas Devlieghere via Phabricator via cfe-commits]

JDevlieghere added a comment.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92597

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

[PATCH] D79773: [clang-format] Improve clang-formats handling of concepts

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay updated this revision to Diff 309350.
MyDeveloperDay marked an inline comment as done.
MyDeveloperDay added a comment.

Address the issue with still munching the semi
mark as experimental in the release notes


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

https://reviews.llvm.org/D79773

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/FormatToken.h
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/lib/Format/UnwrappedLineParser.h
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -14104,6 +14104,7 @@
   CHECK_PARSE_BOOL(BinPackArguments);
   CHECK_PARSE_BOOL(BinPackParameters);
   CHECK_PARSE_BOOL(BreakAfterJavaFieldAnnotations);
+  CHECK_PARSE_BOOL(BreakBeforeConceptDeclarations);
   CHECK_PARSE_BOOL(BreakBeforeTernaryOperators);
   CHECK_PARSE_BOOL(BreakStringLiterals);
   CHECK_PARSE_BOOL(CompactNamespaces);
@@ -14115,6 +14116,7 @@
   CHECK_PARSE_BOOL(IndentCaseLabels);
   CHECK_PARSE_BOOL(IndentCaseBlocks);
   CHECK_PARSE_BOOL(IndentGotoLabels);
+  CHECK_PARSE_BOOL(IndentRequires);
   CHECK_PARSE_BOOL(IndentWrappedFunctionNames);
   CHECK_PARSE_BOOL(KeepEmptyLinesAtTheStartOfBlocks);
   CHECK_PARSE_BOOL(ObjCSpaceAfterProperty);
@@ -17288,6 +17290,277 @@
"}",
Style);
 }
+
+TEST_F(FormatTest, ConceptsAndRequires) {
+  FormatStyle Style = getLLVMStyle();
+  Style.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_None;
+
+  verifyFormat("template \n"
+   "concept Hashable = requires(T a) {\n"
+   "  { std::hash{}(a) } -> std::convertible_to;\n"
+   "};",
+   Style);
+  verifyFormat("template \n"
+   "concept EqualityComparable = requires(T a, T b) {\n"
+   "  { a == b } -> bool;\n"
+   "};",
+   Style);
+  verifyFormat("template \n"
+   "concept EqualityComparable = requires(T a, T b) {\n"
+   "  { a == b } -> bool;\n"
+   "  { a != b } -> bool;\n"
+   "};",
+   Style);
+  verifyFormat("template \n"
+   "concept EqualityComparable = requires(T a, T b) {\n"
+   "  { a == b } -> bool;\n"
+   "  { a != b } -> bool;\n"
+   "};",
+   Style);
+
+  verifyFormat("template \n"
+   "requires Iterator\n"
+   "void sort(It begin, It end) {\n"
+   "  //\n"
+   "}",
+   Style);
+
+  verifyFormat("template \n"
+   "concept Large = sizeof(T) > 10;",
+   Style);
+
+  verifyFormat("template \n"
+   "concept FooableWith = requires(T t, U u) {\n"
+   "  typename T::foo_type;\n"
+   "  { t.foo(u) } -> typename T::foo_type;\n"
+   "  t++;\n"
+   "};\n"
+   "void doFoo(FooableWith auto t) {\n"
+   "  t.foo(3);\n"
+   "}",
+   Style);
+  verifyFormat("template \n"
+   "concept Context = sizeof(T) == 1;",
+   Style);
+  verifyFormat("template \n"
+   "concept Context = is_specialization_of_v;",
+   Style);
+  verifyFormat("template \n"
+   "concept Node = std::is_object_v;",
+   Style);
+  verifyFormat("template \n"
+   "concept Tree = true;",
+   Style);
+
+  verifyFormat("template  int g(T i) requires Concept1 {\n"
+   "  //...\n"
+   "}",
+   Style);
+
+  verifyFormat(
+  "template  int g(T i) requires Concept1 && Concept2 {\n"
+  "  //...\n"
+  "}",
+  Style);
+
+  verifyFormat(
+  "template  int g(T i) requires Concept1 || Concept2 {\n"
+  "  //...\n"
+  "}",
+  Style);
+
+  verifyFormat("template \n"
+   "veryveryvery_long_return_type g(T i) requires Concept1 || "
+   "Concept2 {\n"
+   "  //...\n"
+   "}",
+   Style);
+
+  verifyFormat("template \n"
+   "veryveryvery_long_return_type g(T i) requires Concept1 && "
+   "Concept2 {\n"
+   "  //...\n"
+   "}",
+   Style);
+
+  verifyFormat(
+  "template \n"
+  "veryveryvery_long_return_type g(T i) requires Concept1 && Concept2 {\n"
+  "  //...\n"
+  "}",
+  Style);
+
+  verifyFormat(
+  "template \n"
+  "veryveryvery_long_return_type g(T i) requires Concept1 || Concept2 {\n"
+  "  //...\n"
+  "}",
+  Style);
+
+  verifyFormat("template \n"
+   "requires Foo() && Bar {\n"
+   "  //\n"
+   "}",
+   

[PATCH] D79773: [clang-format] Improve clang-formats handling of concepts

[MyDeveloperDay via Phabricator via cfe-commits]

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



Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2314
+  nextToken();
+  if (FormatTok->Tok.is(tok::less)) {
+while (!FormatTok->Tok.is(tok::greater)) {

klimek wrote:
> MyDeveloperDay wrote:
> > miscco wrote:
> > > miscco wrote:
> > > > I guess you could use `parseBracedList(/*ContinueOnSemicolons=*/false, 
> > > > /*IsEnum=*/false,/*ClosingBraceKind=*/tok::greater);` here?
> > > To be more specific, I am concerned what happens if you have multiple 
> > > template arguments where a linebreak should occur. Is this still 
> > > happening here?
> > > 
> > > 
> > > ```
> > > template 
> > > concept someVeryLongConceptName = 
> > > someVeryLongConstraintName1;
> > > ```
> > This is formatted as
> > 
> > ```
> > template 
> > concept someVeryLongConceptName =
> > someVeryLongConstraintName1;
> > ```
> This seems like a very detailed way to parse them; generally, we try to only 
> parse rough structure here.
So whilst I tend to agree, and I've tried to write this loop to be less and it 
just didn't seem to catch the cases that had already been given in the unit 
tests,

This ended up being a lot likes its own parseStructuralElement, I think the 
difference here is that I've written it as a series of if's rather than a 
switch statement.

I feel I'd be happier to push the patch through then address individual 
specific cases



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

https://reviews.llvm.org/D79773

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

[PATCH] D92602: [objc] diagnose protocol conformance in categories with direct members in their corresponding class interfaces

[Alex Lorenz via Phabricator via cfe-commits]

arphaman created this revision.
arphaman added reviewers: erik.pilkington, MadCoder.
Herald added subscribers: ributzka, jfb, jkorous.
arphaman requested review of this revision.
Herald added a project: clang.

Categories that add protocol conformances to classes with direct members should 
prohibit protocol conformances when the methods/properties that the protocol 
expects are actually declared as 'direct' in the class.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D92602

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDeclObjC.cpp
  clang/test/SemaObjC/category-direct-members-protocol-conformance.m

Index: clang/test/SemaObjC/category-direct-members-protocol-conformance.m
===
--- /dev/null
+++ clang/test/SemaObjC/category-direct-members-protocol-conformance.m
@@ -0,0 +1,98 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+__attribute__((objc_root_class))
+@interface RootClass
+
+- (void)baseMethod;
+
+@end
+
+__attribute__((objc_direct_members))
+@interface I : RootClass
+
+- (void)direct; // expected-note {{direct member declared here}}
+
+@end
+
+@protocol P
+- (void)direct;
+@end
+
+@interface I (Cat1)  // expected-error {{category 'Cat1' cannot conform to protocol 'P' because of direct members declared in interface 'I'}}
+@end
+
+@protocol BaseP
+- (void)baseMethod;
+@end
+
+@interface I (CatBase)  // OK
+@end
+
+@protocol P2
+- (void)indirect;
+@end
+
+@interface I (Cat2)  // OK
+- (void)indirect;
+@end
+
+@protocol P3
+- (void)indirect3;
+@end
+
+@interface I (Cat3)  // OK
+@end
+
+@interface ExpDirect : RootClass
+
+- (void)direct __attribute__((objc_direct)); // expected-note {{direct member declared here}}
+
+- (void)directRecursive __attribute__((objc_direct)); // expected-note {{direct member declared here}}
+
+@end
+
+@interface ExpDirect (CatExpDirect)  // expected-error {{category 'CatExpDirect' cannot conform to protocol 'P' because of direct members declared in interface 'ExpDirect'}}
+@end
+
+@protocol PRecursive1
+- (void)directRecursive;
+@end
+
+@protocol PRecursiveTop 
+@end
+
+@interface ExpDirect ()  // expected-error {{class extension cannot conform to protocol 'PRecursive1' because of direct members declared in interface 'ExpDirect'}}
+@end
+
+
+@protocol PProp
+
+@property (nonatomic, readonly) I *name;
+
+@end
+
+__attribute__((objc_direct_members))
+@interface IProp1 : RootClass
+
+@property (nonatomic, readonly) I *name; // expected-note {{direct member declared here}}
+
+@end
+
+@interface IProp1 ()  // expected-error {{class extension cannot conform to protocol 'PProp' because of direct members declared in interface 'IProp1'}}
+@end
+
+
+@protocol PProp2
+
+@property (nonatomic, readonly, class) I *name;
+
+@end
+
+@interface IProp2 : RootClass
+
+@property (nonatomic, readonly, class, direct) I *name; // expected-note {{direct member declared here}}
+
+@end
+
+@interface IProp2 ()  // expected-error {{class extension cannot conform to protocol 'PProp2' because of direct members declared in interface 'IProp2'}}
+@end
Index: clang/lib/Sema/SemaDeclObjC.cpp
===
--- clang/lib/Sema/SemaDeclObjC.cpp
+++ clang/lib/Sema/SemaDeclObjC.cpp
@@ -3912,6 +3912,55 @@
   }
 }
 
+static void DiagnoseCategoryDirectMembersProtocolConformance(
+Sema , ObjCProtocolDecl *PDecl, ObjCCategoryDecl *CDecl);
+
+static void DiagnoseCategoryDirectMembersProtocolConformance(
+Sema , ObjCCategoryDecl *CDecl,
+const llvm::iterator_range ) {
+  for (auto *PI : Protocols)
+DiagnoseCategoryDirectMembersProtocolConformance(S, PI, CDecl);
+}
+
+static void DiagnoseCategoryDirectMembersProtocolConformance(
+Sema , ObjCProtocolDecl *PDecl, ObjCCategoryDecl *CDecl) {
+  if (!PDecl->isThisDeclarationADefinition() && PDecl->getDefinition())
+PDecl = PDecl->getDefinition();
+
+  llvm::SmallVector DirectMembers;
+  const auto *IDecl = CDecl->getClassInterface();
+  for (auto *MD : PDecl->methods()) {
+if (!MD->isPropertyAccessor()) {
+  if (const auto *CMD =
+  IDecl->getMethod(MD->getSelector(), MD->isInstanceMethod())) {
+if (CMD->isDirectMethod())
+  DirectMembers.push_back(CMD);
+  }
+}
+  }
+  for (auto *PD : PDecl->properties()) {
+if (const auto *CPD = IDecl->FindPropertyVisibleInPrimaryClass(
+PD->getIdentifier(),
+PD->isClassProperty()
+? ObjCPropertyQueryKind::OBJC_PR_query_class
+: ObjCPropertyQueryKind::OBJC_PR_query_instance)) {
+  if (CPD->isDirectProperty())
+DirectMembers.push_back(CPD);
+}
+  }
+  if (!DirectMembers.empty()) {
+S.Diag(CDecl->getLocation(), diag::err_objc_direct_protocol_conformance)
+<< CDecl->IsClassExtension() << CDecl << PDecl << IDecl;
+for (const auto *MD : DirectMembers)
+  S.Diag(MD->getLocation(), diag::note_direct_member_here);
+

[PATCH] D92600: [ASTImporter] Add support for importing GenericSelectionExpr AST nodes.

[Tom Roeder via Phabricator via cfe-commits]

tmroeder updated this revision to Diff 309344.
tmroeder edited the summary of this revision.
tmroeder added a comment.

Fix a typo in the summary.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92600

Files:
  clang/docs/LibASTMatchersReference.html
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/lib/AST/ASTImporter.cpp
  clang/lib/ASTMatchers/ASTMatchersInternal.cpp
  clang/lib/ASTMatchers/Dynamic/Registry.cpp
  clang/lib/Analysis/ExprMutationAnalyzer.cpp
  clang/test/ASTMerge/generic-selection-expr/Inputs/generic.c
  clang/test/ASTMerge/generic-selection-expr/test.c
  clang/unittests/AST/ASTImporterTest.cpp
  clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp

Index: clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
@@ -985,6 +985,11 @@
   EXPECT_TRUE(matches("int* i = __null;", gnuNullExpr()));
 }
 
+TEST_P(ASTMatchersTest, GenericSelectionExpr) {
+  EXPECT_TRUE(matches("void f() { (void)_Generic(1, int: 1, float: 2.0); }",
+  genericSelectionExpr()));
+}
+
 TEST_P(ASTMatchersTest, AtomicExpr) {
   EXPECT_TRUE(matches("void foo() { int *ptr; __atomic_load_n(ptr, 1); }",
   atomicExpr()));
Index: clang/unittests/AST/ASTImporterTest.cpp
===
--- clang/unittests/AST/ASTImporterTest.cpp
+++ clang/unittests/AST/ASTImporterTest.cpp
@@ -279,6 +279,15 @@
  functionDecl(hasDescendant(gnuNullExpr(hasType(isInteger());
 }
 
+TEST_P(ImportExpr, ImportGenericSelectionExpr) {
+  MatchVerifier Verifier;
+
+  testImport(
+  "void declToImport() { int x; (void)_Generic(x, int: 0, float: 1); }",
+  Lang_C99, "", Lang_C99, Verifier,
+  functionDecl(hasDescendant(genericSelectionExpr(;
+}
+
 TEST_P(ImportExpr, ImportCXXNullPtrLiteralExpr) {
   MatchVerifier Verifier;
   testImport(
@@ -1088,6 +1097,36 @@
 ToChooseExpr->isConditionDependent());
 }
 
+TEST_P(ASTImporterOptionSpecificTestBase, ImportGenericSelectionExpr) {
+  Decl *From, *To;
+  std::tie(From, To) = getImportedDecl(
+  R"(
+  int declToImport() {
+int x;
+return _Generic(x, int: 0, default: 1);
+  }
+  )",
+  Lang_C99, "", Lang_C99);
+
+  auto ToResults =
+  match(genericSelectionExpr().bind("expr"), To->getASTContext());
+  auto FromResults =
+  match(genericSelectionExpr().bind("expr"), From->getASTContext());
+
+  const GenericSelectionExpr *FromGenericSelectionExpr =
+  selectFirst("expr", FromResults);
+  ASSERT_TRUE(FromGenericSelectionExpr);
+
+  const GenericSelectionExpr *ToGenericSelectionExpr =
+  selectFirst("expr", ToResults);
+  ASSERT_TRUE(ToGenericSelectionExpr);
+
+  EXPECT_EQ(FromGenericSelectionExpr->isResultDependent(),
+ToGenericSelectionExpr->isResultDependent());
+  EXPECT_EQ(FromGenericSelectionExpr->getResultIndex(),
+ToGenericSelectionExpr->getResultIndex());
+}
+
 TEST_P(ASTImporterOptionSpecificTestBase,
ImportFunctionWithBackReferringParameter) {
   Decl *From, *To;
Index: clang/test/ASTMerge/generic-selection-expr/test.c
===
--- /dev/null
+++ clang/test/ASTMerge/generic-selection-expr/test.c
@@ -0,0 +1,4 @@
+// RUN: %clang_cc1 -std=c11 -emit-pch -o %t.ast %S/Inputs/generic.c
+// RUN: %clang_cc1 -std=c11 -ast-merge %t.ast -fsyntax-only -verify %s
+// expected-no-diagnostics
+
Index: clang/test/ASTMerge/generic-selection-expr/Inputs/generic.c
===
--- /dev/null
+++ clang/test/ASTMerge/generic-selection-expr/Inputs/generic.c
@@ -0,0 +1,7 @@
+void f() {
+  int x;
+  float y;
+  _Static_assert(_Generic(x, float: 0, int: 1), "Incorrect semantics of _Generic");
+  _Static_assert(_Generic(y, float: 1, int: 0), "Incorrect semantics of _Generic");
+}
+
Index: clang/lib/Analysis/ExprMutationAnalyzer.cpp
===
--- clang/lib/Analysis/ExprMutationAnalyzer.cpp
+++ clang/lib/Analysis/ExprMutationAnalyzer.cpp
@@ -95,10 +95,6 @@
   return Node.isPotentiallyEvaluated();
 }
 
-const ast_matchers::internal::VariadicDynCastAllOfMatcher
-genericSelectionExpr;
-
 AST_MATCHER_P(GenericSelectionExpr, hasControllingExpr,
   ast_matchers::internal::Matcher, InnerMatcher) {
   return InnerMatcher.matches(*Node.getControllingExpr(), Finder, Builder);
Index: clang/lib/ASTMatchers/Dynamic/Registry.cpp
===
--- clang/lib/ASTMatchers/Dynamic/Registry.cpp
+++ clang/lib/ASTMatchers/Dynamic/Registry.cpp
@@ -241,6 +241,7 @@
   REGISTER_MATCHER(functionProtoType);
   REGISTER_MATCHER(functionTemplateDecl);

[PATCH] D91980: [OpenMP] Add initial support for `omp [begin/end] assumes`

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added inline comments.



Comment at: clang/lib/Sema/SemaLambda.cpp:999-1002
+  // OpenMP lambdas might get assumumption attributes.
+  if (LangOpts.OpenMP)
+ActOnFinishedFunctionDefinitionInOpenMPAssumeScope(Method);
+

ABataev wrote:
> Are there any other function-like constructs that also should have markings? 
> Coroutines maybe? 
Hm, can we postpone coroutines for now?



Comment at: clang/lib/Sema/SemaOpenMP.cpp:3203
+  auto *AA = AssumptionAttr::Create(Context, llvm::join(Assumptions, ","), 
Loc);
+  // Disable assumes in OpenMP simd mode.
+  if (DKind == llvm::omp::Directive::OMPD_begin_assumes) {

ABataev wrote:
> How this comment relates to the code?
Leftover.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:3207
+  } else {
+assert(DKind == llvm::omp::Directive::OMPD_assumes && "");
+OMPAssumeGlobal.push_back(AA);

ABataev wrote:
> Add a message in for the assertion
Good catch!



Comment at: clang/lib/Sema/SemaOpenMP.cpp:3217
+while (Ctx->getParent())
+  Ctx = Ctx->getParent();
+DeclContexts.push_back(Ctx);

ABataev wrote:
> Maybe, better to use `getLexicalParent`? `getParent` returns semantic parent, 
> while `getLexicalParent` - the lexical parent. Do you need to mark the 
> declarations in the lexical context or in the semantic context?
I go to the top, should not matter. I want all declarations so I start at the 
top most scope, then traverse everything. I go with lexical now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91980

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

[PATCH] D91980: [OpenMP] Add initial support for `omp [begin/end] assumes`

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert updated this revision to Diff 309340.
jdoerfert marked 4 inline comments as done.
jdoerfert added a comment.

Addressed comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91980

Files:
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Parse/ParseOpenMP.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaLambda.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/assumes_codegen.cpp
  clang/test/OpenMP/assumes_include_nvptx.cpp
  clang/test/OpenMP/assumes_messages.c
  clang/test/OpenMP/assumes_print.cpp
  clang/test/OpenMP/assumes_template_print.cpp
  llvm/include/llvm/Frontend/OpenMP/OMP.td
  llvm/include/llvm/Frontend/OpenMP/OMPConstants.h
  llvm/include/llvm/Frontend/OpenMP/OMPKinds.def

Index: llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
===
--- llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
+++ llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
@@ -1224,3 +1224,27 @@
 #undef __OMP_REQUIRES_TRAIT
 #undef OMP_REQUIRES_TRAIT
 ///}
+
+
+/// Assumption clauses
+///
+///{
+
+#ifdef OMP_ASSUME_CLAUSE
+#define __OMP_ASSUME_CLAUSE(Identifier, StartsWith, HasDirectiveList, HasExpression) \
+OMP_ASSUME_CLAUSE(Identifier, StartsWith, HasDirectiveList, HasExpression)
+#else
+#define __OMP_ASSUME_CLAUSE(...)
+#endif
+
+__OMP_ASSUME_CLAUSE("ext_", true, false, false)
+__OMP_ASSUME_CLAUSE("absent", false, true, false)
+__OMP_ASSUME_CLAUSE("contains", false, true, false)
+__OMP_ASSUME_CLAUSE("holds", false, false, true)
+__OMP_ASSUME_CLAUSE("no_openmp", false, false, false)
+__OMP_ASSUME_CLAUSE("no_openmp_routines", false, false, false)
+__OMP_ASSUME_CLAUSE("no_parallelism", false, false, false)
+
+#undef __OMP_ASSUME_CLAUSE
+#undef OMP_ASSUME_CLAUSE
+///}
Index: llvm/include/llvm/Frontend/OpenMP/OMPConstants.h
===
--- llvm/include/llvm/Frontend/OpenMP/OMPConstants.h
+++ llvm/include/llvm/Frontend/OpenMP/OMPConstants.h
@@ -16,6 +16,7 @@
 
 #include "llvm/ADT/BitmaskEnum.h"
 
+#include "llvm/ADT/StringRef.h"
 #include "llvm/Frontend/OpenMP/OMP.h.inc"
 
 namespace llvm {
@@ -79,6 +80,33 @@
 #define OMP_IDENT_FLAG(Enum, ...) constexpr auto Enum = omp::IdentFlag::Enum;
 #include "llvm/Frontend/OpenMP/OMPKinds.def"
 
+/// Helper to describe assume clauses.
+struct AssumptionClauseMappingInfo {
+  /// The identifier describing the (beginning of the) clause.
+  llvm::StringLiteral Identifier;
+  /// Flag to determine if the identifier is a full name or the start of a name.
+  bool StartsWith;
+  /// Flag to determine if a directive lists follows.
+  bool HasDirectiveList;
+  /// Flag to determine if an expression follows.
+  bool HasExpression;
+};
+
+/// All known assume clauses.
+static constexpr AssumptionClauseMappingInfo AssumptionClauseMappings[] = {
+#define OMP_ASSUME_CLAUSE(Identifier, StartsWith, HasDirectiveList,\
+  HasExpression)   \
+  {Identifier, StartsWith, HasDirectiveList, HasExpression},
+#include "llvm/Frontend/OpenMP/OMPKinds.def"
+};
+
+inline std::string getAllAssumeClauseOptions() {
+  std::string S;
+  for (const AssumptionClauseMappingInfo  : AssumptionClauseMappings)
+S += (S.empty() ? "'" : "', '") + ACMI.Identifier.str();
+  return S + "'";
+}
+
 } // end namespace omp
 
 } // end namespace llvm
Index: llvm/include/llvm/Frontend/OpenMP/OMP.td
===
--- llvm/include/llvm/Frontend/OpenMP/OMP.td
+++ llvm/include/llvm/Frontend/OpenMP/OMP.td
@@ -1590,6 +1590,9 @@
 VersionedClause
   ];
 }
+def OMP_Assumes : Directive<"assumes"> {}
+def OMP_BeginAssumes : Directive<"begin assumes"> {}
+def OMP_EndAssumes : Directive<"end assumes"> {}
 def OMP_BeginDeclareVariant : Directive<"begin declare variant"> {}
 def OMP_EndDeclareVariant : Directive<"end declare variant"> {}
 def OMP_ParallelWorkshare : Directive<"parallel workshare"> {
Index: clang/test/OpenMP/assumes_template_print.cpp
===
--- /dev/null
+++ clang/test/OpenMP/assumes_template_print.cpp
@@ -0,0 +1,91 @@
+// RUN: %clang_cc1 -verify -fopenmp -ast-print %s | FileCheck %s
+// RUN: %clang_cc1 -fopenmp -x c++ -std=c++11 -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp -std=c++11 -include-pch %t -fsyntax-only -verify %s -ast-print | FileCheck %s
+
+// RUN: %clang_cc1 -verify -fopenmp-simd -ast-print %s | FileCheck %s
+// RUN: %clang_cc1 -fopenmp-simd -x c++ -std=c++11 -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp-simd -std=c++11 -include-pch %t -fsyntax-only -verify %s -ast-print | FileCheck %s
+// expected-no-diagnostics
+
+// It is unclear if we want to annotate the template instantiations, e.g., S::foo, or not in the two
+// 

[PATCH] D91949: [clang-format] Add BeforeStructInitialization option in BraceWrapping configuration

[Anastasiia Lukianenko via Phabricator via cfe-commits]

anastasiia_lukianenko updated this revision to Diff 309336.
anastasiia_lukianenko retitled this revision from "[clang-format] Add 
BreakBeforeStructInitialization configuration" to "[clang-format] Add 
BeforeStructInitialization option in BraceWrapping configuration".
anastasiia_lukianenko added reviewers: djasper, klimek, alexfh, mprobst.
anastasiia_lukianenko added a comment.

1. Configuration became BraceWrapping option
2. Added CHECK_PARSE_NESTED_BOOL test
3. Fixed bugs with unwanted formatting




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

https://reviews.llvm.org/D91949

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/ContinuationIndenter.cpp
  clang/lib/Format/Format.cpp
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -5046,6 +5046,25 @@
 format(Input, Style));
 }
 
+TEST_F(FormatTest, BreakBeforeStructInitialization) {
+  FormatStyle Style = getLLVMStyle();
+  Style.ColumnLimit = 80;
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+  Style.BraceWrapping.BeforeStructInitialization = true;
+  verifyFormat("struct new_struct struct_name =\n"
+   "{\n"
+   "a = 1,\n"
+   "b = 2,\n"
+   "};",
+   Style);
+  Style.BraceWrapping.BeforeStructInitialization = false;
+  verifyFormat("struct new_struct struct_name = {\n"
+   "a = 1,\n"
+   "b = 2,\n"
+   "};",
+   Style);
+}
+
 TEST_F(FormatTest, BreakConstructorInitializersAfterColon) {
   FormatStyle Style = getLLVMStyle();
   Style.BreakConstructorInitializers = FormatStyle::BCIS_AfterColon;
@@ -14154,6 +14173,7 @@
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, BeforeCatch);
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, BeforeElse);
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, BeforeLambdaBody);
+  CHECK_PARSE_NESTED_BOOL(BraceWrapping, BeforeStructInitialization);
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, BeforeWhile);
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, IndentBraces);
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, SplitEmptyFunction);
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -3489,6 +3489,10 @@
 bool TokenAnnotator::mustBreakBefore(const AnnotatedLine ,
  const FormatToken ) {
   const FormatToken  = *Right.Previous;
+  if (Style.BraceWrapping.BeforeStructInitialization &&
+  Line.First->is(tok::kw_struct) && Right.is(tok::l_brace) &&
+  (Right.is(BK_BracedInit) || Left.is(tok::equal)))
+return true;
   if (Right.NewlinesBefore > 1 && Style.MaxEmptyLinesToKeep > 0)
 return true;
 
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -657,6 +657,8 @@
 IO.mapOptional("BeforeCatch", Wrapping.BeforeCatch);
 IO.mapOptional("BeforeElse", Wrapping.BeforeElse);
 IO.mapOptional("BeforeLambdaBody", Wrapping.BeforeLambdaBody);
+IO.mapOptional("BeforeStructInitialization",
+   Wrapping.BeforeStructInitialization);
 IO.mapOptional("BeforeWhile", Wrapping.BeforeWhile);
 IO.mapOptional("IndentBraces", Wrapping.IndentBraces);
 IO.mapOptional("SplitEmptyFunction", Wrapping.SplitEmptyFunction);
@@ -754,6 +756,7 @@
 /*BeforeCatch=*/false,
 /*BeforeElse=*/false,
 /*BeforeLambdaBody=*/false,
+/*BeforeStructInitialization=*/false,
 /*BeforeWhile=*/false,
 /*IndentBraces=*/false,
 /*SplitEmptyFunction=*/true,
@@ -887,6 +890,7 @@
  /*BeforeCatch=*/false,
  /*BeforeElse=*/false,
  /*BeforeLambdaBody=*/false,
+ /*BeforeStructInitialization=*/false,
  /*BeforeWhile=*/false,
  /*IndentBraces=*/false,
  /*SplitEmptyFunction=*/true,
Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -972,8 +972,10 @@
   const FormatToken  = *Current.Previous;
   // If we are continuing an expression, we want to use the continuation indent.
   unsigned ContinuationIndent =
-  std::max(State.Stack.back().LastSpace, State.Stack.back().Indent) +
-  

[PATCH] D92257: [clang-format] Add option to control the space at the front of a line comment

[Marek Kurdej via Phabricator via cfe-commits]

curdeius accepted this revision.
curdeius added a comment.

LGTM




Comment at: clang/lib/Format/Format.cpp:963
   LLVMStyle.SpacesInCStyleCastParentheses = false;
+  LLVMStyle.SpacesInLineComments = {/*Minimum=*/1, /*Maximum=*/-1u};
   LLVMStyle.SpaceAfterCStyleCast = false;

HazardyKnusperkeks wrote:
> curdeius wrote:
> > I don't know precisely the LLVM style but does it allow more than one space 
> > (as Maximum would suggest)?
> > Are there any tests covering that?
> > And what about other styles, no need to set min/max for them?
> The part with the LLVM Style from my test case did run exactly so without any 
> modification, so yes it allows more than one space.
> 
> Since there was no option before (that I'm aware of) all other styles behaved 
> exactly like that. I did not check the style guides if they say anything 
> about that, I just preserved the old behavior when nothing is configured.
Ok. Great


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92257

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

[PATCH] D87216: [NewPM] Support --print-before/after in NPM

[Arthur Eubanks via Phabricator via cfe-commits]

aeubanks updated this revision to Diff 309338.
aeubanks added a comment.

forward declare class


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87216

Files:
  llvm/include/llvm/IR/IRPrintingPasses.h
  llvm/include/llvm/IR/PassInstrumentation.h
  llvm/include/llvm/IR/PrintPasses.h
  llvm/include/llvm/Passes/StandardInstrumentations.h
  llvm/lib/Analysis/CallGraphSCCPass.cpp
  llvm/lib/Analysis/LoopInfo.cpp
  llvm/lib/Analysis/LoopPass.cpp
  llvm/lib/CodeGen/MachineFunctionPrinterPass.cpp
  llvm/lib/IR/CMakeLists.txt
  llvm/lib/IR/IRPrintingPasses.cpp
  llvm/lib/IR/LegacyPassManager.cpp
  llvm/lib/IR/PassInstrumentation.cpp
  llvm/lib/IR/PrintPasses.cpp
  llvm/lib/Passes/PassBuilder.cpp
  llvm/lib/Passes/StandardInstrumentations.cpp
  llvm/test/CodeGen/Generic/print-after.ll
  llvm/test/Other/loop-pass-printer.ll
  llvm/test/Other/print-before-after.ll
  llvm/utils/gn/secondary/llvm/lib/IR/BUILD.gn

Index: llvm/utils/gn/secondary/llvm/lib/IR/BUILD.gn
===
--- llvm/utils/gn/secondary/llvm/lib/IR/BUILD.gn
+++ llvm/utils/gn/secondary/llvm/lib/IR/BUILD.gn
@@ -59,6 +59,7 @@
 "PassManager.cpp",
 "PassRegistry.cpp",
 "PassTimingInfo.cpp",
+"PrintPasses.cpp",
 "ProfileSummary.cpp",
 "SafepointIRVerifier.cpp",
 "Statepoint.cpp",
Index: llvm/test/Other/print-before-after.ll
===
--- /dev/null
+++ llvm/test/Other/print-before-after.ll
@@ -0,0 +1,33 @@
+; RUN: not --crash opt < %s -disable-output -passes='no-op-module' -print-before=bleh 2>&1 | FileCheck %s --check-prefix=NONE --allow-empty
+; RUN: not --crash opt < %s -disable-output -passes='no-op-module' -print-after=bleh 2>&1 | FileCheck %s --check-prefix=NONE --allow-empty
+; RUN: opt < %s -disable-output -passes='no-op-module' -print-before=no-op-function 2>&1 | FileCheck %s --check-prefix=NONE --allow-empty
+; RUN: opt < %s -disable-output -passes='no-op-module' -print-after=no-op-function 2>&1 | FileCheck %s --check-prefix=NONE --allow-empty
+; RUN: opt < %s -disable-output -passes='no-op-module,no-op-function' -print-before=no-op-module 2>&1 | FileCheck %s --check-prefix=ONCE
+; RUN: opt < %s -disable-output -passes='no-op-module,no-op-function' -print-after=no-op-module 2>&1 | FileCheck %s --check-prefix=ONCE
+; RUN: opt < %s -disable-output -passes='no-op-function' -print-before=no-op-function 2>&1 | FileCheck %s --check-prefix=ONCE
+; RUN: opt < %s -disable-output -passes='no-op-function' -print-after=no-op-function 2>&1 | FileCheck %s --check-prefix=ONCE
+; RUN: opt < %s -disable-output -passes='no-op-module,no-op-function' -print-before=no-op-function --print-module-scope 2>&1 | FileCheck %s --check-prefix=TWICE
+; RUN: opt < %s -disable-output -passes='no-op-module,no-op-function' -print-after=no-op-function --print-module-scope 2>&1 | FileCheck %s --check-prefix=TWICE
+
+; NONE-NOT: @foo
+; NONE-NOT: @bar
+
+; ONCE: @foo
+; ONCE: @bar
+; ONCE-NOT: @foo
+; ONCE-NOT: @bar
+
+; TWICE: @foo
+; TWICE: @bar
+; TWICE: @foo
+; TWICE: @bar
+; TWICE-NOT: @foo
+; TWICE-NOT: @bar
+
+define void @foo() {
+  ret void
+}
+
+define void @bar() {
+  ret void
+}
Index: llvm/test/Other/loop-pass-printer.ll
===
--- llvm/test/Other/loop-pass-printer.ll
+++ llvm/test/Other/loop-pass-printer.ll
@@ -1,23 +1,23 @@
 ; This test checks -print-after/before on loop passes
 ; Besides of the loop itself it should be dumping loop pre-header and exits.
 ;
-; RUN: opt -enable-new-pm=0 < %s 2>&1 -disable-output \
+; RUN: opt < %s 2>&1 -disable-output \
 ; RUN: 	   -loop-deletion -print-before=loop-deletion \
 ; RUN:	   | FileCheck %s -check-prefix=DEL
 ; RUN: opt < %s 2>&1 -disable-output \
-; RUN: 	   -passes='loop(loop-deletion)' -print-before-all \
+; RUN: 	   -passes='loop(loop-deletion)' -print-before=loop-deletion \
 ; RUN:	   | FileCheck %s -check-prefix=DEL
 ; RUN: opt -enable-new-pm=0 < %s 2>&1 -disable-output \
 ; RUN: 	   -loop-unroll -print-after=loop-unroll -filter-print-funcs=bar \
 ; RUN:	   | FileCheck %s -check-prefix=BAR -check-prefix=BAR-OLD
 ; RUN: opt < %s 2>&1 -disable-output \
-; RUN: 	   -passes='require,loop(loop-unroll-full)' -print-after-all -filter-print-funcs=bar \
+; RUN: 	   -passes='require,loop(loop-unroll-full)' -print-after=loop-unroll-full -filter-print-funcs=bar \
 ; RUN:	   | FileCheck %s -check-prefix=BAR
 ; RUN: opt -enable-new-pm=0 < %s 2>&1 -disable-output \
 ; RUN: 	   -loop-unroll -print-after=loop-unroll -filter-print-funcs=foo -print-module-scope \
 ; RUN:	   | FileCheck %s -check-prefix=FOO-MODULE -check-prefix=FOO-MODULE-OLD
 ; RUN: opt < %s 2>&1 -disable-output \
-; RUN: 	   -passes='require,loop(loop-unroll-full)' -print-after-all -filter-print-funcs=foo -print-module-scope \
+; RUN: 	   -passes='require,loop(loop-unroll-full)' 

[PATCH] D92600: [ASTImporter] Add support for importing GenericSelectionExpr AST nodes.

[Tom Roeder via Phabricator via cfe-commits]

tmroeder created this revision.
tmroeder added reviewers: aaron.ballman, klimek.
Herald added subscribers: teemperor, martong.
Herald added a reviewer: a.sidorin.
Herald added a reviewer: shafik.
tmroeder requested review of this revision.
Herald added a project: clang.

This allows ASTs to be merged when they contain GenericSelectionExpr
nodes (this is _Generic from C11). This is needed, for example, for
cross-CTU analysis of C code that makes use of _Generic, like the Linux
kernel.

The node is already supported in the AST, but it didn't have a matcher
in ASTMatchers. So, this change adds the matcher and adds support to
ASTImporter.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D92600

Files:
  clang/docs/LibASTMatchersReference.html
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/lib/AST/ASTImporter.cpp
  clang/lib/ASTMatchers/ASTMatchersInternal.cpp
  clang/lib/ASTMatchers/Dynamic/Registry.cpp
  clang/lib/Analysis/ExprMutationAnalyzer.cpp
  clang/test/ASTMerge/generic-selection-expr/Inputs/generic.c
  clang/test/ASTMerge/generic-selection-expr/test.c
  clang/unittests/AST/ASTImporterTest.cpp
  clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp

Index: clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
@@ -985,6 +985,11 @@
   EXPECT_TRUE(matches("int* i = __null;", gnuNullExpr()));
 }
 
+TEST_P(ASTMatchersTest, GenericSelectionExpr) {
+  EXPECT_TRUE(matches("void f() { (void)_Generic(1, int: 1, float: 2.0); }",
+  genericSelectionExpr()));
+}
+
 TEST_P(ASTMatchersTest, AtomicExpr) {
   EXPECT_TRUE(matches("void foo() { int *ptr; __atomic_load_n(ptr, 1); }",
   atomicExpr()));
Index: clang/unittests/AST/ASTImporterTest.cpp
===
--- clang/unittests/AST/ASTImporterTest.cpp
+++ clang/unittests/AST/ASTImporterTest.cpp
@@ -279,6 +279,15 @@
  functionDecl(hasDescendant(gnuNullExpr(hasType(isInteger());
 }
 
+TEST_P(ImportExpr, ImportGenericSelectionExpr) {
+  MatchVerifier Verifier;
+
+  testImport(
+  "void declToImport() { int x; (void)_Generic(x, int: 0, float: 1); }",
+  Lang_C99, "", Lang_C99, Verifier,
+  functionDecl(hasDescendant(genericSelectionExpr(;
+}
+
 TEST_P(ImportExpr, ImportCXXNullPtrLiteralExpr) {
   MatchVerifier Verifier;
   testImport(
@@ -1088,6 +1097,36 @@
 ToChooseExpr->isConditionDependent());
 }
 
+TEST_P(ASTImporterOptionSpecificTestBase, ImportGenericSelectionExpr) {
+  Decl *From, *To;
+  std::tie(From, To) = getImportedDecl(
+  R"(
+  int declToImport() {
+int x;
+return _Generic(x, int: 0, default: 1);
+  }
+  )",
+  Lang_C99, "", Lang_C99);
+
+  auto ToResults =
+  match(genericSelectionExpr().bind("expr"), To->getASTContext());
+  auto FromResults =
+  match(genericSelectionExpr().bind("expr"), From->getASTContext());
+
+  const GenericSelectionExpr *FromGenericSelectionExpr =
+  selectFirst("expr", FromResults);
+  ASSERT_TRUE(FromGenericSelectionExpr);
+
+  const GenericSelectionExpr *ToGenericSelectionExpr =
+  selectFirst("expr", ToResults);
+  ASSERT_TRUE(ToGenericSelectionExpr);
+
+  EXPECT_EQ(FromGenericSelectionExpr->isResultDependent(),
+ToGenericSelectionExpr->isResultDependent());
+  EXPECT_EQ(FromGenericSelectionExpr->getResultIndex(),
+ToGenericSelectionExpr->getResultIndex());
+}
+
 TEST_P(ASTImporterOptionSpecificTestBase,
ImportFunctionWithBackReferringParameter) {
   Decl *From, *To;
Index: clang/test/ASTMerge/generic-selection-expr/test.c
===
--- /dev/null
+++ clang/test/ASTMerge/generic-selection-expr/test.c
@@ -0,0 +1,4 @@
+// RUN: %clang_cc1 -std=c11 -emit-pch -o %t.ast %S/Inputs/generic.c
+// RUN: %clang_cc1 -std=c11 -ast-merge %t.ast -fsyntax-only -verify %s
+// expected-no-diagnostics
+
Index: clang/test/ASTMerge/generic-selection-expr/Inputs/generic.c
===
--- /dev/null
+++ clang/test/ASTMerge/generic-selection-expr/Inputs/generic.c
@@ -0,0 +1,7 @@
+void f() {
+  int x;
+  float y;
+  _Static_assert(_Generic(x, float: 0, int: 1), "Incorrect semantics of _Generic");
+  _Static_assert(_Generic(y, float: 1, int: 0), "Incorrect semantics of _Generic");
+}
+
Index: clang/lib/Analysis/ExprMutationAnalyzer.cpp
===
--- clang/lib/Analysis/ExprMutationAnalyzer.cpp
+++ clang/lib/Analysis/ExprMutationAnalyzer.cpp
@@ -95,10 +95,6 @@
   return Node.isPotentiallyEvaluated();
 }
 
-const ast_matchers::internal::VariadicDynCastAllOfMatcher
-genericSelectionExpr;
-
 AST_MATCHER_P(GenericSelectionExpr, hasControllingExpr,
   

[PATCH] D79773: [clang-format] Improve clang-formats handling of concepts

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay updated this revision to Diff 309335.
MyDeveloperDay marked an inline comment as done.
MyDeveloperDay added a comment.

rebase before making any further changes


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

https://reviews.llvm.org/D79773

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/FormatToken.h
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/lib/Format/UnwrappedLineParser.h
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -14104,6 +14104,7 @@
   CHECK_PARSE_BOOL(BinPackArguments);
   CHECK_PARSE_BOOL(BinPackParameters);
   CHECK_PARSE_BOOL(BreakAfterJavaFieldAnnotations);
+  CHECK_PARSE_BOOL(BreakBeforeConceptDeclarations);
   CHECK_PARSE_BOOL(BreakBeforeTernaryOperators);
   CHECK_PARSE_BOOL(BreakStringLiterals);
   CHECK_PARSE_BOOL(CompactNamespaces);
@@ -14115,6 +14116,7 @@
   CHECK_PARSE_BOOL(IndentCaseLabels);
   CHECK_PARSE_BOOL(IndentCaseBlocks);
   CHECK_PARSE_BOOL(IndentGotoLabels);
+  CHECK_PARSE_BOOL(IndentRequires);
   CHECK_PARSE_BOOL(IndentWrappedFunctionNames);
   CHECK_PARSE_BOOL(KeepEmptyLinesAtTheStartOfBlocks);
   CHECK_PARSE_BOOL(ObjCSpaceAfterProperty);
@@ -17288,6 +17290,277 @@
"}",
Style);
 }
+
+TEST_F(FormatTest, ConceptsAndRequires) {
+  FormatStyle Style = getLLVMStyle();
+  Style.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_None;
+
+  verifyFormat("template \n"
+   "concept Hashable = requires(T a) {\n"
+   "  { std::hash{}(a) } -> std::convertible_to;\n"
+   "};",
+   Style);
+  verifyFormat("template \n"
+   "concept EqualityComparable = requires(T a, T b) {\n"
+   "  { a == b } -> bool;\n"
+   "};",
+   Style);
+  verifyFormat("template \n"
+   "concept EqualityComparable = requires(T a, T b) {\n"
+   "  { a == b } -> bool;\n"
+   "  { a != b } -> bool;\n"
+   "};",
+   Style);
+  verifyFormat("template \n"
+   "concept EqualityComparable = requires(T a, T b) {\n"
+   "  { a == b } -> bool;\n"
+   "  { a != b } -> bool;\n"
+   "};",
+   Style);
+
+  verifyFormat("template \n"
+   "requires Iterator\n"
+   "void sort(It begin, It end) {\n"
+   "  //\n"
+   "}",
+   Style);
+
+  verifyFormat("template \n"
+   "concept Large = sizeof(T) > 10;",
+   Style);
+
+  verifyFormat("template \n"
+   "concept FooableWith = requires(T t, U u) {\n"
+   "  typename T::foo_type;\n"
+   "  { t.foo(u) } -> typename T::foo_type;\n"
+   "  t++;\n"
+   "};\n"
+   "void doFoo(FooableWith auto t) {\n"
+   "  t.foo(3);\n"
+   "}",
+   Style);
+  verifyFormat("template \n"
+   "concept Context = sizeof(T) == 1;",
+   Style);
+  verifyFormat("template \n"
+   "concept Context = is_specialization_of_v;",
+   Style);
+  verifyFormat("template \n"
+   "concept Node = std::is_object_v;",
+   Style);
+  verifyFormat("template \n"
+   "concept Tree = true;",
+   Style);
+
+  verifyFormat("template  int g(T i) requires Concept1 {\n"
+   "  //...\n"
+   "}",
+   Style);
+
+  verifyFormat(
+  "template  int g(T i) requires Concept1 && Concept2 {\n"
+  "  //...\n"
+  "}",
+  Style);
+
+  verifyFormat(
+  "template  int g(T i) requires Concept1 || Concept2 {\n"
+  "  //...\n"
+  "}",
+  Style);
+
+  verifyFormat("template \n"
+   "veryveryvery_long_return_type g(T i) requires Concept1 || "
+   "Concept2 {\n"
+   "  //...\n"
+   "}",
+   Style);
+
+  verifyFormat("template \n"
+   "veryveryvery_long_return_type g(T i) requires Concept1 && "
+   "Concept2 {\n"
+   "  //...\n"
+   "}",
+   Style);
+
+  verifyFormat(
+  "template \n"
+  "veryveryvery_long_return_type g(T i) requires Concept1 && Concept2 {\n"
+  "  //...\n"
+  "}",
+  Style);
+
+  verifyFormat(
+  "template \n"
+  "veryveryvery_long_return_type g(T i) requires Concept1 || Concept2 {\n"
+  "  //...\n"
+  "}",
+  Style);
+
+  verifyFormat("template \n"
+   "requires Foo() && Bar {\n"
+   "  //\n"
+   "}",
+   Style);
+
+  verifyFormat("template \n"
+   

[PATCH] D92257: [clang-format] Add option to control the space at the front of a line comment

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

HazardyKnusperkeks added inline comments.



Comment at: clang/lib/Format/Format.cpp:963
   LLVMStyle.SpacesInCStyleCastParentheses = false;
+  LLVMStyle.SpacesInLineComments = {/*Minimum=*/1, /*Maximum=*/-1u};
   LLVMStyle.SpaceAfterCStyleCast = false;

curdeius wrote:
> I don't know precisely the LLVM style but does it allow more than one space 
> (as Maximum would suggest)?
> Are there any tests covering that?
> And what about other styles, no need to set min/max for them?
The part with the LLVM Style from my test case did run exactly so without any 
modification, so yes it allows more than one space.

Since there was no option before (that I'm aware of) all other styles behaved 
exactly like that. I did not check the style guides if they say anything about 
that, I just preserved the old behavior when nothing is configured.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92257

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

[PATCH] D92257: [clang-format] Add option to control the space at the front of a line comment

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added inline comments.



Comment at: clang/lib/Format/Format.cpp:963
   LLVMStyle.SpacesInCStyleCastParentheses = false;
+  LLVMStyle.SpacesInLineComments = {/*Minimum=*/1, /*Maximum=*/-1u};
   LLVMStyle.SpaceAfterCStyleCast = false;

I don't know precisely the LLVM style but does it allow more than one space (as 
Maximum would suggest)?
Are there any tests covering that?
And what about other styles, no need to set min/max for them?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92257

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

[PATCH] D92363: [HIP] Warn no --offload-arch option

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

In D92363#2431812 , @yaxunl wrote:

> 



> There are still valid use cases for not providing GPU arch. For example, 
> users would like to test syntax of a HIP program for which GPU arch does not 
> matter, or users would like to get PCH file where GPU arch does not matter. 
> Another example is cmake may test whether a compiler can compile HIP program 
> without options. Make it a warning allows such uses cases.

Issuing no warning allows such use cases even better. :-)

Also, how do I silence that warning, if I'm compiling with -Werror? 
If we provide a knob to silence the warning, how would that be better than 
asking the user to specify a GPU arch? 
If we do ask users to specify GPU arch in order to silence the warning => we 
effectively make the GPU arch a mandatory option.

I believe that no warning or a mandatory GPU arch would be a more principled 
way. With the warning we effectively flag the default value as 'it's not a very 
useful default'. If it's not useful by default, then it should not be the 
default. If it is useful, there should be no warning.

In this case, I think making GPU arch mandatory would have a stronger case, as 
the default we use right now is indeed quite often wrong. However, we probably 
have existing users who have valid use cases that work without the explicitly 
specified GPU arch, so requiring the GPU arch would break them. That constrains 
us to the no-warning scenario in practice.

Introducing a warning would break existing users, too -- compiling with 
`-Werror` is not that uncommon. If we're willing to break users, we may as well 
break them by requiring a GPU arch.
I just don't see a compelling argument where adding a warning would provide a 
net benefit over no-warning or a mandatory-GPU-arch scenarios.


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

https://reviews.llvm.org/D92363

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

[PATCH] D92597: ARCMigrate: Initialize fields in EditEntry inline, NFC

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith created this revision.
dexonsmith added reviewers: jansvoboda11, JDevlieghere, akyrtzi.
Herald added a subscriber: ributzka.
dexonsmith requested review of this revision.
Herald added a project: clang.

Initialize the fields inline instead of having to manually write out a
default constructor.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D92597

Files:
  clang/lib/ARCMigrate/ObjCMT.cpp


Index: clang/lib/ARCMigrate/ObjCMT.cpp
===
--- clang/lib/ARCMigrate/ObjCMT.cpp
+++ clang/lib/ARCMigrate/ObjCMT.cpp
@@ -2034,12 +2034,10 @@
 
 namespace {
 struct EditEntry {
-  const FileEntry *File;
-  unsigned Offset;
-  unsigned RemoveLen;
+  const FileEntry *File = nullptr;
+  unsigned Offset = 0;
+  unsigned RemoveLen = 0;
   std::string Text;
-
-  EditEntry() : File(), Offset(), RemoveLen() {}
 };
 } // end anonymous namespace
 


Index: clang/lib/ARCMigrate/ObjCMT.cpp
===
--- clang/lib/ARCMigrate/ObjCMT.cpp
+++ clang/lib/ARCMigrate/ObjCMT.cpp
@@ -2034,12 +2034,10 @@
 
 namespace {
 struct EditEntry {
-  const FileEntry *File;
-  unsigned Offset;
-  unsigned RemoveLen;
+  const FileEntry *File = nullptr;
+  unsigned Offset = 0;
+  unsigned RemoveLen = 0;
   std::string Text;
-
-  EditEntry() : File(), Offset(), RemoveLen() {}
 };
 } // end anonymous namespace
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D92596: [FPEnv] Correct constrained metadata in fp16-ops-strict.c

[Kevin P. Neal via Phabricator via cfe-commits]

kpn created this revision.
kpn added reviewers: sepavloff, mibintc.
kpn added a project: clang.
kpn requested review of this revision.
Herald added a subscriber: cfe-commits.

This test shows we're in some cases not getting strictfp information from the 
AST. Correct that.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D92596

Files:
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/test/CodeGen/fp16-ops-strictfp.c

Index: clang/test/CodeGen/fp16-ops-strictfp.c
===
--- clang/test/CodeGen/fp16-ops-strictfp.c
+++ clang/test/CodeGen/fp16-ops-strictfp.c
@@ -11,7 +11,6 @@
 //
 // Test that the constrained intrinsics are picking up the exception
 // metadata from the AST instead of the global default from the command line.
-// FIXME: All cases of "fpexcept.maytrap" in this test are wrong.
 
 #pragma float_control(except, on)
 
@@ -62,31 +61,31 @@
   // NOTNATIVE: store {{.*}} half {{.*}}, half*
   h1 = +h1;
 
-  // NATIVE-HALF: call half @llvm.experimental.constrained.fadd.f16(half %{{.*}}, half 0xH3C00, metadata !"round.tonearest", metadata !"fpexcept.maytrap")
-  // NOTNATIVE: call float @llvm.experimental.constrained.fpext.f32.f16(half %{{.*}}, metadata !"fpexcept.maytrap")
-  // NOTNATIVE: call float @llvm.experimental.constrained.fadd.f32(float %{{.*}}, float {{.*}}, metadata !"round.tonearest", metadata !"fpexcept.maytrap")
-  // NOTNATIVE: call half @llvm.experimental.constrained.fptrunc.f16.f32(float %{{.*}}, metadata !"round.tonearest", metadata !"fpexcept.maytrap")
+  // NATIVE-HALF: call half @llvm.experimental.constrained.fadd.f16(half %{{.*}}, half 0xH3C00, metadata !"round.tonearest", metadata !"fpexcept.strict")
+  // NOTNATIVE: call float @llvm.experimental.constrained.fpext.f32.f16(half %{{.*}}, metadata !"fpexcept.strict")
+  // NOTNATIVE: call float @llvm.experimental.constrained.fadd.f32(float %{{.*}}, float {{.*}}, metadata !"round.tonearest", metadata !"fpexcept.strict")
+  // NOTNATIVE: call half @llvm.experimental.constrained.fptrunc.f16.f32(float %{{.*}}, metadata !"round.tonearest", metadata !"fpexcept.strict")
   // CHECK: store {{.*}} half {{.*}}, half*
   h1++;
 
-  // NATIVE-HALF: call half @llvm.experimental.constrained.fadd.f16(half %{{.*}}, half 0xH3C00, metadata !"round.tonearest", metadata !"fpexcept.maytrap")
-  // NOTNATIVE: call float @llvm.experimental.constrained.fpext.f32.f16(half %{{.*}}, metadata !"fpexcept.maytrap")
-  // NOTNATIVE: call float @llvm.experimental.constrained.fadd.f32(float %{{.*}}, float {{.*}}, metadata !"round.tonearest", metadata !"fpexcept.maytrap")
-  // NOTNATIVE: call half @llvm.experimental.constrained.fptrunc.f16.f32(float %{{.*}}, metadata !"round.tonearest", metadata !"fpexcept.maytrap")
+  // NATIVE-HALF: call half @llvm.experimental.constrained.fadd.f16(half %{{.*}}, half 0xH3C00, metadata !"round.tonearest", metadata !"fpexcept.strict")
+  // NOTNATIVE: call float @llvm.experimental.constrained.fpext.f32.f16(half %{{.*}}, metadata !"fpexcept.strict")
+  // NOTNATIVE: call float @llvm.experimental.constrained.fadd.f32(float %{{.*}}, float {{.*}}, metadata !"round.tonearest", metadata !"fpexcept.strict")
+  // NOTNATIVE: call half @llvm.experimental.constrained.fptrunc.f16.f32(float %{{.*}}, metadata !"round.tonearest", metadata !"fpexcept.strict")
   // CHECK: store {{.*}} half {{.*}}, half*
   ++h1;
 
-  // NATIVE-HALF: call half @llvm.experimental.constrained.fadd.f16(half %{{.*}}, half 0xHBC00, metadata !"round.tonearest", metadata !"fpexcept.maytrap")
-  // NOTNATIVE: call float @llvm.experimental.constrained.fpext.f32.f16(half %{{.*}}, metadata !"fpexcept.maytrap")
-  // NOTNATIVE: call float @llvm.experimental.constrained.fadd.f32(float %{{.*}}, float {{.*}}, metadata !"round.tonearest", metadata !"fpexcept.maytrap")
-  // NOTNATIVE: call half @llvm.experimental.constrained.fptrunc.f16.f32(float %{{.*}}, metadata !"round.tonearest", metadata !"fpexcept.maytrap")
+  // NATIVE-HALF: call half @llvm.experimental.constrained.fadd.f16(half %{{.*}}, half 0xHBC00, metadata !"round.tonearest", metadata !"fpexcept.strict")
+  // NOTNATIVE: call float @llvm.experimental.constrained.fpext.f32.f16(half %{{.*}}, metadata !"fpexcept.strict")
+  // NOTNATIVE: call float @llvm.experimental.constrained.fadd.f32(float %{{.*}}, float {{.*}}, metadata !"round.tonearest", metadata !"fpexcept.strict")
+  // NOTNATIVE: call half @llvm.experimental.constrained.fptrunc.f16.f32(float %{{.*}}, metadata !"round.tonearest", metadata !"fpexcept.strict")
   // CHECK: store {{.*}} half {{.*}}, half*
   --h1;
 
-  // NATIVE-HALF: call half @llvm.experimental.constrained.fadd.f16(half %{{.*}}, half 0xHBC00, metadata !"round.tonearest", metadata !"fpexcept.maytrap")
-  // NOTNATIVE: call float @llvm.experimental.constrained.fpext.f32.f16(half %{{.*}}, metadata !"fpexcept.maytrap")
-  // NOTNATIVE: call float @llvm.experimental.constrained.fadd.f32(float %{{.*}}, float {{.*}}, metadata 

[PATCH] D92257: [clang-format] Add option to control the space at the front of a line comment

[MyDeveloperDay via Phabricator via cfe-commits]

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

This LGTM, I'm not sure if others have any comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92257

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

[PATCH] D87216: [NewPM] Support --print-before/after in NPM

[Yuanfang Chen via Phabricator via cfe-commits]

ychen added a comment.

In D87216#2431626 , @aeubanks wrote:

> In D87216#2431508 , @ychen wrote:
>
>> It is very unfortunate that we have to manage and translate between two sets 
>> of names (one pass name and one type name). This makes me wonder if we just 
>> keep the pass name as the return value of PassInfoMixin::name and get rid of 
>> class name everywhere. Right now I couldn't think of anything is blocking us 
>> from doing that. WDYT?  @asbirlea ?
>
> We'd have to move the names from PassRegistry.def to every pass class 
> definition which would be a lot of work but definitely feasible.

That's true. The issue of translation also happens for codegen using NPM where 
both target-dependent and target-independent passes need the translation. 
Looking at my prototype, there is a lot of boilerplate for that. I think the 
one-time cost of moving names around should be worthwhile.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87216

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

[PATCH] D92579: [clangd] AddUsing: Fix a crash on ElaboratedTypes without NestedNameSpecfiiers.

[Adam Czachorowski via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGc282b7de5a5d: [clangd] AddUsing: Fix a crash on 
ElaboratedTypes without NestedNameSpecfiiers. (authored by adamcz).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92579

Files:
  clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
  clang-tools-extra/clangd/unittests/TweakTests.cpp


Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -2523,6 +2523,9 @@
   // Do not offer code action on typo-corrections.
   EXPECT_UNAVAILABLE(Header + "/*error-ok*/c^c C;");
 
+  // NestedNameSpecifier, but no namespace.
+  EXPECT_UNAVAILABLE(Header + "class Foo {}; class F^oo foo;");
+
   // Check that we do not trigger in header files.
   FileName = "test.h";
   ExtraArgs.push_back("-xc++-header"); // .h file is treated a C by default.
Index: clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
@@ -274,6 +274,8 @@
   } else if (auto *T = Node->ASTNode.get()) {
 if (auto E = T->getAs()) {
   QualifierToRemove = E.getQualifierLoc();
+  if (!QualifierToRemove)
+return false;
 
   auto SpelledTokens =
   TB.spelledForExpanded(TB.expandedTokens(E.getSourceRange()));


Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -2523,6 +2523,9 @@
   // Do not offer code action on typo-corrections.
   EXPECT_UNAVAILABLE(Header + "/*error-ok*/c^c C;");
 
+  // NestedNameSpecifier, but no namespace.
+  EXPECT_UNAVAILABLE(Header + "class Foo {}; class F^oo foo;");
+
   // Check that we do not trigger in header files.
   FileName = "test.h";
   ExtraArgs.push_back("-xc++-header"); // .h file is treated a C by default.
Index: clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
@@ -274,6 +274,8 @@
   } else if (auto *T = Node->ASTNode.get()) {
 if (auto E = T->getAs()) {
   QualifierToRemove = E.getQualifierLoc();
+  if (!QualifierToRemove)
+return false;
 
   auto SpelledTokens =
   TB.spelledForExpanded(TB.expandedTokens(E.getSourceRange()));
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] c282b7d - [clangd] AddUsing: Fix a crash on ElaboratedTypes without NestedNameSpecfiiers.

[Adam Czachorowski via cfe-commits]

Author: Adam Czachorowski
Date: 2020-12-03T20:25:38+01:00
New Revision: c282b7de5a5de8151a19228702867e2299f1d3fe

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

LOG: [clangd] AddUsing: Fix a crash on ElaboratedTypes without 
NestedNameSpecfiiers.

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

Added: 


Modified: 
clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
clang-tools-extra/clangd/unittests/TweakTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp 
b/clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
index b00c2716005c7..d6a57efeeef13 100644
--- a/clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
+++ b/clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
@@ -274,6 +274,8 @@ bool AddUsing::prepare(const Selection ) {
   } else if (auto *T = Node->ASTNode.get()) {
 if (auto E = T->getAs()) {
   QualifierToRemove = E.getQualifierLoc();
+  if (!QualifierToRemove)
+return false;
 
   auto SpelledTokens =
   TB.spelledForExpanded(TB.expandedTokens(E.getSourceRange()));

diff  --git a/clang-tools-extra/clangd/unittests/TweakTests.cpp 
b/clang-tools-extra/clangd/unittests/TweakTests.cpp
index eefc50d754e2a..07f061b3f2e39 100644
--- a/clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ b/clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -2523,6 +2523,9 @@ class cc {
   // Do not offer code action on typo-corrections.
   EXPECT_UNAVAILABLE(Header + "/*error-ok*/c^c C;");
 
+  // NestedNameSpecifier, but no namespace.
+  EXPECT_UNAVAILABLE(Header + "class Foo {}; class F^oo foo;");
+
   // Check that we do not trigger in header files.
   FileName = "test.h";
   ExtraArgs.push_back("-xc++-header"); // .h file is treated a C by default.



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

[PATCH] D92363: [HIP] Warn no --offload-arch option

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

In D92363#2426401 , @tra wrote:

> While I agree that the default GPU choice is not likely to be correct, or 
> usable, for everyone, but the warning seems to be a half-measure.
> If the default is not usable, then it should not be the default. If it's 
> usable, then we don't need a warning.
>
> Having a warning would make sense if it were a part of the plan to transition 
> towards making GPU arch a mandatory option. Is that the case here?
> Just a warning is not very useful, IMO. The users would need to specify the 
> GPU in order to silence it, so why not just require it.

There are still valid use cases for not providing GPU arch. For example, users 
would like to test syntax of a HIP program for which GPU arch does not matter, 
or users would like to get PCH file where GPU arch does not matter. Another 
example is cmake may test whether a compiler can compile HIP program without 
options. Make it a warning allows such uses cases.


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

https://reviews.llvm.org/D92363

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

[PATCH] D92480: [llvm] Unify interface of (ThreadSafe)?RefCountedBase

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith added inline comments.



Comment at: llvm/include/llvm/ADT/IntrusiveRefCntPtr.h:75-76
   RefCountedBase() = default;
+  // Copy and move constructors/assignments are no-ops as the RefCount isn't
+  // dictated by the class directly.
   RefCountedBase(const RefCountedBase &) {}

njames93 wrote:
> njames93 wrote:
> > dexonsmith wrote:
> > > dblaikie wrote:
> > > > I can't quite understand this comment - perhaps you could try 
> > > > rephrasing it?
> > > I'm not quite following why you needed to add these special members; it 
> > > seems like just the destructor would be enough for the assertion; but if 
> > > you do need them, can/should they be `= default`?
> > They can't be defaulted as we don't want to copy the RefCount.
> I understand it in my head, just can't think of the best way to write it down.
Maybe something like this would work:
```
/// Do not copy or move RefCount when constructing or assigning from
/// a derived type. These operations don't imply anything about the
/// number of IntrusiveRefCntPtr instances.
```
WDYT?



Comment at: llvm/include/llvm/ADT/IntrusiveRefCntPtr.h:75-80
+  // Copy and move constructors/assignments are no-ops as the RefCount isn't
+  // dictated by the class directly.
   RefCountedBase(const RefCountedBase &) {}
+  RefCountedBase(RefCountedBase &&) {}
+  RefCountedBase =(const RefCountedBase &) { return *this; }
+  RefCountedBase =(RefCountedBase &&) { return *this; }

dexonsmith wrote:
> njames93 wrote:
> > njames93 wrote:
> > > dexonsmith wrote:
> > > > dblaikie wrote:
> > > > > I can't quite understand this comment - perhaps you could try 
> > > > > rephrasing it?
> > > > I'm not quite following why you needed to add these special members; it 
> > > > seems like just the destructor would be enough for the assertion; but 
> > > > if you do need them, can/should they be `= default`?
> > > They can't be defaulted as we don't want to copy the RefCount.
> > I understand it in my head, just can't think of the best way to write it 
> > down.
> Maybe something like this would work:
> ```
> /// Do not copy or move RefCount when constructing or assigning from
> /// a derived type. These operations don't imply anything about the
> /// number of IntrusiveRefCntPtr instances.
> ```
> WDYT?
Ah, I get it, thanks for explaining.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92480

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

[PATCH] D92570: [clang] [Headers] Use the corresponding _aligned_free or __mingw_aligned_free in _mm_free

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added inline comments.



Comment at: clang/lib/Headers/mm_malloc.h:42
   void *__mallocedMemory;
 #if defined(__MINGW32__)
   __mallocedMemory = __mingw_aligned_malloc(__size, __align);

rnk wrote:
> This has been here since 2011, and somehow nobody complained that we didn't 
> use the corresponding `__mingw_aligned_free` API...
Due to 
https://github.com/mingw-w64/mingw-w64/blob/master/mingw-w64-headers/crt/malloc.h#L72
 (which is included implicitly from stdlib.h), it hasn't been used much at all- 
maybe only when using the older mingw.org headers. But I'm considering 
suggesting to change that bit in the mingw-w64 headers (it's a bit of a mess 
when used with gcc's mm_malloc.h).

For msvc based configs it would have been noticable though. But I don't know 
how much `_mm_malloc` really is used...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92570

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

[PATCH] D90173: [PowerPC] Exploit splat instruction xxsplti32dx in Power10

[Albion Fung via Phabricator via cfe-commits]

Conanap added a comment.

addressed a comment




Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:9366
+SplatNode = DAG.getNode(
+PPCISD::XXSPLTI32DX, SDLoc(SplatNode), MVT::v2i64, SplatNode,
+DAG.getTargetConstant(isLE ? 1 : 0, SplatNode, MVT::i32),

amyk wrote:
> I think I'm still a little confused by this. Do we not need `dl` when we do 
> `getNode()` here?
Hey so the DL is supplied from `SplatNode`'s definition, and then we pass in 
`SDLoc(SplatNode)` here as the proper SDLoc after new instr def.


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

https://reviews.llvm.org/D90173

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

[PATCH] D92231: [OpenCL] Implement extended subgroups fully in headers

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

In D92231#2425323 , @Anastasia wrote:

> In D92231#2425305 , @PiotrFusik 
> wrote:
>
>> The specification for these extensions was edited by Ben Ashbaugh @Intel. 
>> I've asked him about this change.
>
> Sure. Thanks! Happy to hold off committing for a few days to see if Ben has 
> any feedback.

FYI, I have discussed this with Ben offline and he doesn't seem to have any 
objections on the approach. I will keep this open for another week to see if 
there is any detailed feedback from anyone.


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

https://reviews.llvm.org/D92231

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

[PATCH] D87216: [NewPM] Support --print-before/after in NPM

[Arthur Eubanks via Phabricator via cfe-commits]

aeubanks added a comment.

In D87216#2431508 , @ychen wrote:

> It is very unfortunate that we have to manage and translate between two sets 
> of names (one pass name and one type name). This makes me wonder if we just 
> keep the pass name as the return value of PassInfoMixin::name and get rid of 
> class name everywhere. Right now I couldn't think of anything is blocking us 
> from doing that. WDYT?  @asbirlea ?

We'd have to move the names from PassRegistry.def to every pass class 
definition which would be a lot of work but definitely feasible.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87216

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

[PATCH] D92209: [ASTImporter] Support CXXDeductionGuideDecl with local typedef

[Gabor Marton via Phabricator via cfe-commits]

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

> I was thinking about to create a separate ASTImporter implementation 
> specifically for CTU and LLDB could have it's own implementation. For that we 
> just need to create an interface class with virtual functions.

One implementation could reside in libCrossTU and the other in LLDB. At least, 
that's what I thought. Unfortunately there is an obstacle which renders this 
whole idea practically unfeasible: Currently ASTImporter and ASTNodeImporter 
are `friend`s of almost all AST classes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92209

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

[PATCH] D92427: [OPENMP51] Add present modifier in defaultmap clause

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:3490
+OMPC_MAP_MODIFIER_present);
+ImplicitMapModifierLoc[ClauseKind].push_back(SourceLocation());
+  }

cchen wrote:
> ABataev wrote:
> > Why need to add an empty `SourceLocation`?
> I'm doing this since there is no actual location for present modifier. Maybe 
> I should just pass llvm::None to ActOnOpenMPMapClause.
Could you store the original location of the `present` modifier and add a real 
source location rather. Better to use correct locations when possible, it may 
help with the debug info emission


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92427

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

[PATCH] D91944: OpenMP 5.0 metadirective

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added inline comments.



Comment at: clang/include/clang/Serialization/ASTBitCodes.h:1952
-  STMT_MS_DEPENDENT_EXISTS,   // MSDependentExistsStmt
-  EXPR_LAMBDA,// LambdaExpr
   STMT_COROUTINE_BODY,

alokmishra.besu wrote:
> jdoerfert wrote:
> > Unrelated.
> Only STMT_OMP_META_DIRECTIVE was added. Rest was formatted by git clang-format
> Rest was formatted by git clang-format

Please don't format unrelated code. 



Comment at: clang/lib/Parse/ParseOpenMP.cpp:2264
+  Actions.StartOpenMPDSABlock(DirKind, DirName, Actions.getCurScope(),
+  Loc);
+  int paren = 0;

alokmishra.besu wrote:
> jdoerfert wrote:
> > Should we not go back to the original code handling "directives" instead? 
> > This looks like it is copied here.
> Unfortunately we cannot go to the original code handling since the original 
> code handling assumes that the directive always ends with 
> annot_pragma_openmp_end, while here it will always end with ')'.
> In specification 5.0, since we are choosing only 1 directive, the body of the 
> while block remains the same as the original code. Only the condition of the 
> while block changes. In specification 5.1, we will need to generate code for 
> dynamic handling and even the body will differ as we might need to generate 
> AST node for multiple directives. It is best if we handle this code here for 
> easier handling of 5.1 code, than in the original code space.
> I will add a TODO comment here.
> Unfortunately we cannot go to the original code handling since the original 
> code handling assumes that the directive always ends with 
> annot_pragma_openmp_end, while here it will always end with ')'.

Let's add a flag to the original handling to make this possible then. Copying 
it is going to create more long term problems.



Comment at: llvm/lib/Frontend/OpenMP/OMPContext.cpp:337
+const SmallVectorImpl , const OMPContext ,
+SmallVectorImpl *OrderedMatch) {
+

alokmishra.besu wrote:
> jdoerfert wrote:
> > This looks like a clone of `getBestVariantMatchForContext` with an extra 
> > unused argument.
> I intended to keep it similar for 5.0 to be updated in 5.1 code. But anyways 
> it seems to be giving wrong result. Will go through this again.
Use the `getBestVariantMatchForContext` interface now and for the 5.1 semantics 
we introduce the `OrderedMatch`. That can also be used in the call site declare 
variant handling which currently calls `getBestVariantMatchForContext` multiple 
times.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91944

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

[PATCH] D91944: OpenMP 5.0 metadirective

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/include/clang/AST/StmtOpenMP.h:373
+///
+class OMPMetaDirective final : public OMPExecutableDirective {
+  friend class ASTStmtReader;

alokmishra.besu wrote:
> ABataev wrote:
> > I think, metadirective should be a kind of a container for different 
> > sub-directives. The problem is that that subdirectives could be completely 
> > different, they may capture different variables, using different capture 
> > kind (by value or by reference) etc.So, you need to generate each possible 
> > sub-directive independently and store them in the meta directive node. 
> > Otherwise you won't be able to generate the code correctly.
> In OpenMP 5.0, we do not need to generate every sub-directive. Rather we need 
> to select one (or none) directive and replace metadirective with it. So this 
> is not needed.
> Yes with future specifications we will need to include a list of all valid 
> directives which need to be resolved at runtime. That is when we will need to 
> generate and store multiple sub-directives inside the OMPMetaDirective class. 
>  
I think you still need to do it even for 5.0. I don't mean y need to emit the 
code for every sub-directive, but to generate them in the AST. Even in the 
example above, `when(user={condition(N>10)}` is used in the template and `N` is 
a template argument, the chosen directive can be selected on the instantiation 
and, thus, the original templated directive has to contain all possible 
sub-directives.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91944

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

[PATCH] D87216: [NewPM] Support --print-before/after in NPM

[Yuanfang Chen via Phabricator via cfe-commits]

ychen accepted this revision.
ychen added a comment.

It is very unfortunate that we have to manage and translate between two sets of 
names (one pass name and one type name). This makes me wonder if we just keep 
the pass name as the return value of PassInfoMixin::name and get rid of class 
name everywhere. Right now I couldn't think of anything is blocking us from 
doing that. WDYT?  @asbirlea ?




Comment at: llvm/include/llvm/Passes/StandardInstrumentations.h:24
 #include "llvm/IR/ValueHandle.h"
+#include "llvm/Passes/PassBuilder.h"
 #include "llvm/Support/CommandLine.h"

It is better if forward declaring PassInstrumentationCallbacks could remove 
this include.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87216

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

[PATCH] D92427: [OPENMP51] Add present modifier in defaultmap clause

[Chi Chun Chen via Phabricator via cfe-commits]

cchen updated this revision to Diff 309292.
cchen added a comment.

Update based on comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92427

Files:
  clang/include/clang/Basic/OpenMPKinds.def
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/target_ast_print.cpp
  clang/test/OpenMP/target_defaultmap_codegen.cpp
  clang/test/OpenMP/target_defaultmap_codegen_01.cpp
  clang/test/OpenMP/target_defaultmap_codegen_02.cpp
  clang/test/OpenMP/target_defaultmap_messages.cpp

Index: clang/test/OpenMP/target_defaultmap_messages.cpp
===
--- clang/test/OpenMP/target_defaultmap_messages.cpp
+++ clang/test/OpenMP/target_defaultmap_messages.cpp
@@ -1,3 +1,6 @@
+// RUN: %clang_cc1 -verify -fopenmp -fopenmp-version=51 %s -verify=expected,omp51 -Wuninitialized -DOMP51
+// RUN: %clang_cc1 -verify -fopenmp-simd -fopenmp-version=51 %s -verify=expected,omp51 -Wuninitialized -DOMP51
+
 // RUN: %clang_cc1 -verify -fopenmp -fopenmp-version=50 %s -verify=expected,omp5 -Wuninitialized -DOMP5
 // RUN: %clang_cc1 -verify -fopenmp-simd -fopenmp-version=50 %s -verify=expected,omp5 -Wuninitialized -DOMP5
 
@@ -27,23 +30,24 @@
 T tmain(T argc, S **argv) {
   #pragma omp target defaultmap // expected-error {{expected '(' after 'defaultmap'}}
   foo();
-#pragma omp target defaultmap( // omp5-error {{expected 'alloc', 'from', 'to', 'tofrom', 'firstprivate', 'none', 'default' in OpenMP clause 'defaultmap'}} expected-error {{expected ')'}} expected-note {{to match this '('}} omp45-error {{expected 'tofrom' in OpenMP clause 'defaultmap'}}
+#pragma omp target defaultmap( // omp51-error {{expected 'alloc', 'from', 'to', 'tofrom', 'firstprivate', 'none', 'default', 'present' in OpenMP clause 'defaultmap'}} omp5-error {{expected 'alloc', 'from', 'to', 'tofrom', 'firstprivate', 'none', 'default' in OpenMP clause 'defaultmap'}} expected-error {{expected ')'}} expected-note {{to match this '('}} omp45-error {{expected 'tofrom' in OpenMP clause 'defaultmap'}}
   foo();
-#pragma omp target defaultmap() // omp5-error {{expected 'alloc', 'from', 'to', 'tofrom', 'firstprivate', 'none', 'default' in OpenMP clause 'defaultmap'}} omp45-error {{expected 'tofrom' in OpenMP clause 'defaultmap'}}
+#pragma omp target defaultmap() // omp51-error {{expected 'alloc', 'from', 'to', 'tofrom', 'firstprivate', 'none', 'default', 'present' in OpenMP clause 'defaultmap'}} omp5-error {{expected 'alloc', 'from', 'to', 'tofrom', 'firstprivate', 'none', 'default' in OpenMP clause 'defaultmap'}} omp45-error {{expected 'tofrom' in OpenMP clause 'defaultmap'}}
   foo();
 #pragma omp target defaultmap(tofrom // expected-error {{expected ')'}} expected-note {{to match this '('}} omp45-warning {{missing ':' after defaultmap modifier - ignoring}} omp45-error {{expected 'scalar' in OpenMP clause 'defaultmap'}}
   foo();
-  #pragma omp target defaultmap (tofrom: // expected-error {{expected ')'}} expected-note {{to match this '('}} omp5-error {{expected 'scalar', 'aggregate', 'pointer' in OpenMP clause 'defaultmap'}} omp45-error {{expected 'scalar' in OpenMP clause 'defaultmap'}}
+  #pragma omp target defaultmap (tofrom: // expected-error {{expected ')'}} expected-note {{to match this '('}} omp51-error {{expected 'scalar', 'aggregate', 'pointer' in OpenMP clause 'defaultmap'}} omp5-error {{expected 'scalar', 'aggregate', 'pointer' in OpenMP clause 'defaultmap'}} omp45-error {{expected 'scalar' in OpenMP clause 'defaultmap'}}
   foo();
 #pragma omp target defaultmap(tofrom) // omp45-warning {{missing ':' after defaultmap modifier - ignoring}} omp45-error {{expected 'scalar' in OpenMP clause 'defaultmap'}}
   foo();
 #pragma omp target defaultmap(tofrom, // expected-error {{expected ')'}} omp45-warning {{missing ':' after defaultmap modifier - ignoring}} expected-note {{to match this '('}} omp45-error {{expected 'scalar' in OpenMP clause 'defaultmap'}}
   foo();
-  #pragma omp target defaultmap (scalar: // omp5-error {{expected 'scalar', 'aggregate', 'pointer' in OpenMP clause 'defaultmap'}} expected-error {{expected ')'}} omp5-error {{expected 'alloc', 'from', 'to', 'tofrom', 'firstprivate', 'none', 'default' in OpenMP clause 'defaultmap'}} expected-note {{to match this '('}} omp45-error {{expected 'tofrom' in OpenMP clause 'defaultmap'}}
+  #pragma omp target defaultmap (scalar: // omp51-error {{expected 'alloc', 'from', 'to', 'tofrom', 'firstprivate', 'none', 'default', 'present' in OpenMP clause 'defaultmap'}} omp51-error {{expected 'scalar', 'aggregate', 'pointer' in OpenMP clause 'defaultmap'}} omp5-error {{expected 'scalar', 'aggregate', 'pointer' in OpenMP clause 'defaultmap'}} expected-error {{expected ')'}} omp5-error {{expected 'alloc', 'from', 'to', 'tofrom', 'firstprivate', 'none', 'default' in OpenMP clause 'defaultmap'}} expected-note {{to match this '('}} omp45-error {{expected 'tofrom' in OpenMP clause 'defaultmap'}}

[PATCH] D91944: OpenMP 5.0 metadirective

[Alok Mishra via Phabricator via cfe-commits]

alokmishra.besu added inline comments.



Comment at: clang/include/clang/AST/StmtOpenMP.h:373
+///
+class OMPMetaDirective final : public OMPExecutableDirective {
+  friend class ASTStmtReader;

ABataev wrote:
> I think, metadirective should be a kind of a container for different 
> sub-directives. The problem is that that subdirectives could be completely 
> different, they may capture different variables, using different capture kind 
> (by value or by reference) etc.So, you need to generate each possible 
> sub-directive independently and store them in the meta directive node. 
> Otherwise you won't be able to generate the code correctly.
In OpenMP 5.0, we do not need to generate every sub-directive. Rather we need 
to select one (or none) directive and replace metadirective with it. So this is 
not needed.
Yes with future specifications we will need to include a list of all valid 
directives which need to be resolved at runtime. That is when we will need to 
generate and store multiple sub-directives inside the OMPMetaDirective class.  


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91944

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

[PATCH] D91885: [clang-tidy] Add support for diagnostics with no location

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang-tools-extra/clang-tidy/ClangTidyCheck.h:183
+
+  /// Adds a diagnostic to report errors in the checks configuration.
+  DiagnosticBuilder

checks -> check's



Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp:276
+  // Never ignore these.
+} else if (!Context.isCheckEnabled(Error.DiagnosticName) &&
+   Error.DiagLevel != ClangTidyError::Error) {

Perhaps this is a bad idea, but would it make sense to have `isCheckEnabled()` 
report `true` for `clang-tidy-config`? It's not really a check, so that's a bit 
odd, but it seems like anywhere we're testing this property we wouldn't want to 
ignore config issues?



Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h:102
+
+  /// Report any errors to do with reading configuration using this method
+  DiagnosticBuilder

with reading configuration -> with reading the configuration

and add a full stop to the end of the comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91885

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

[PATCH] D92570: [clang] [Headers] Use the corresponding _aligned_free or __mingw_aligned_free in _mm_free

[Reid Kleckner via Phabricator via cfe-commits]

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

lgtm




Comment at: clang/lib/Headers/mm_malloc.h:42
   void *__mallocedMemory;
 #if defined(__MINGW32__)
   __mallocedMemory = __mingw_aligned_malloc(__size, __align);

This has been here since 2011, and somehow nobody complained that we didn't use 
the corresponding `__mingw_aligned_free` API...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92570

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

[PATCH] D92024: [clang] Implement P0692R1 from C++20 (access checking on specializations and instantiations)

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

Quuxplusone added a comment.

The subject line says "access checking on specializations and instantiations," 
but I don't see any tests for explicit instantiations here — just 
specializations. In particular, I'm very interested to know if P0692 is 
intended to have any effect on the legality of https://godbolt.org/z/fqfo8q

  template struct T {};
  template struct T<::private_static_data>;

It would also be good to document which of the two proposed wordings from 
http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2017/p0692r1.html is 
actually being adopted in this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92024

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

[PATCH] D92495: [clang] Add a new nullability annotation for swift async: _Nullable_result

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/include/clang/Basic/AttrDocs.td:3516
+``_Nullable_result`` pointer can be ``nil``, just like ``_Nullable``. Where 
this
+attribute differs from ``_Nullable`` is when its used on a parameter to a
+completion handler in a Swift async method. For instance, here:

its -> it's



Comment at: clang/include/clang/Basic/AttrDocs.td:3526
+available, or calls it with an error. ``_Nullable_result`` indicates to the
+Swift imported that this is the uncommon case where ``result`` can get ``nil``
+even if no error has occured, and will therefore import it as a Swift optional

Swift imported -> Swift importer



Comment at: clang/lib/Basic/IdentifierTable.cpp:718-719
+  case NullabilityKind::NullableResult:
+assert(!isContextSensitive &&
+   "_Nullable_result isn't supported as context-sensitive keyword");
+return "_Nullable_result";

Can you explain why it differs from `_Nullable` in this case?



Comment at: clang/lib/Sema/SemaExprObjC.cpp:1566
   unsigned receiverNullabilityIdx = 0;
-  if (auto nullability = ReceiverType->getNullability(Context))
+  if (auto nullability = ReceiverType->getNullability(Context)) {
+if (*nullability == NullabilityKind::NullableResult)

Should that be `auto *`?



Comment at: clang/lib/Sema/SemaExprObjC.cpp:1573
   unsigned resultNullabilityIdx = 0;
-  if (auto nullability = resultType->getNullability(Context))
+  if (auto nullability = resultType->getNullability(Context)) {
+if (*nullability == NullabilityKind::NullableResult)

Same here.


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

https://reviews.llvm.org/D92495

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

[PATCH] D92209: [ASTImporter] Support CXXDeductionGuideDecl with local typedef

[Gabor Marton via Phabricator via cfe-commits]

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



Comment at: clang/lib/AST/ASTImporter.cpp:2522
+  // Add to the lookupTable because we could not do that in MapImported.
+  Importer.AddToLookupTable(ToTypedef);
+

shafik wrote:
> I am not super excited about this solution, I feel like several bugs we have 
> had can be attributed to these exception control flow cases that we have in 
> the ASTImporter. I don't have any immediate better solution.
> 
> 
Before uploading this patch I had been thinking about several other solutions
but all of them had some problems:

1) 
Detect the loop in the AST and return with UnsupportedConstruct. We could do
the detection with the help of ImportPath. However, I realized this approach is
way too defensive and removes the support for an important AST node which is
against our goals.

2) 
Try to eliminate the looping contruct from the AST. I could do that for some
cases (D92101) but there are still cases when the Sema must create such a loop
deliberately (the test case in this patch shows that).

It is essential to add a newly created Decl to the lookupTable ASAP because it
makes it possible for subsequent imports to find the existing Decl and this way
avoiding the creation of duplicated Decls. This is why AddToLookupTable is
called in MapImported. But the lookuptable operates on the DeclContext of the
Decl, and in this patch we must not import the DeclContext before.
Consequently, we have to add to the loopkuptable once the DeclContext is
imported. Perhaps, we could provide an optional lambda function to
GetImportedOrCreateDecl which would be called before MapImported (and that
lambda would do the DC import), but this seems even more clumsy.

BTW, the lookuptable is not used in LLDB, there you use the traditional lookup
implemented in DeclContext (addDeclInternal and noload_lookup). One problem
with noload_lookup is that it does not find some Decls because it obeys to C++
visibility rules, thus new duplicated Decls are created. The ASTImporter must
be able to lookup e.g. template specialization Decls too, in order to avoid
creating a redundant duplicated clone and this is the task of the lookupTable.
I hope one day LLDB would be able to switch to ASTImporterLookupTable from
noload_lookup. The other problem is with addDeclInternal: we call this later,
not in MapImported. The only responsibility attached to the use of 
addDeclInternal 
should be to clone the visibility as it is in the source context (and not 
managing 
the do-not-create-duplicate-decls issue).

So yes, there are many exceptional control flow cases, but most of them stems
from the complexity of trying to support two different needs: noload_lookup and
minimal import are needed exceptionally for LLDB. I was thinking about to
create a separate ASTImporter implementation specifically for CTU and LLDB
could have it's own implementation. For that we just need to create an
interface class with virtual functions. Having two different implementations
could save us from braking each others tests and this could be a big gain, WDYT?
(On the other hand, we'd loose updates and fixes from the other team.)



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92209

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