[PATCH] D88498: [FPEnv] Evaluate initializers in constant rounding mode

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff added inline comments.



Comment at: clang/lib/Parse/ParseDecl.cpp:2290
+  // rounding mode.
+  if (VD->isFileVarDecl() || VD->isConstexpr() ||
+  (!getLangOpts().CPlusPlus && VD->isStaticLocal())) {

sepavloff wrote:
> rsmith wrote:
> > It's far from clear to me that this is correct in C++. In principle, for a 
> > dynamic initializer, the rounding mode could have been set by an earlier 
> > initializer.
> > 
> > Perhaps we can make an argument that, due to the permission to interleave 
> > initializers from different TUs, every dynamic initializer must leave the 
> > program in the default rounding mode, but I don't think even that makes 
> > this approach correct, because an initializer could do this:
> > 
> > ```
> > double d;
> > double e = (fesetround(...), d = some calculation, 
> > fesetround(...default...), d);
> > ```
> > 
> > I think we can only do this in C and will need something different for C++.
> > 
> > (I think this also has problems in C++ due to constexpr functions: it's not 
> > the case that all floating point operations that will be evaluated as part 
> > of the initializer lexically appear within it.)
> I changed the code to confine it with C and constexpr only. Hopefully this 
> would be enough to enable build of SPECS attempted by @mibintc 
> (https://reviews.llvm.org/D87528#2295015). However in long-term perspective 
> we should return to this code.
> 
> The intent was to align behavior of C and C++. If an initializer is valid in 
> C, then it should produce the same code in C++. If the source code like
> ```
> float appx_coeffs_fp32[3 * 256] = {
> SEGMENT_BIAS + 1.4426950216,
> …
> ```
> produces compact table in C mode and huge initialization code in C++, it 
> would be strange from user viewpoint and would not give any advantage.
> 
> C in C2x presents pretty consistent model, provided that `#pragma STDC 
> FENV_ROUND FE_DYNAMIC` does not set constant rounding mode. Initializers for 
> variables with static allocation are always evaluated in constant rounding 
> mode and user can chose the mode using pragma FENV_ROUND.
> 
> When extending this model to C++ we must solve the problem of dynamic 
> initialization. It obviously occurs in runtime rounding mode, so changing 
> between static and dynamic initialization may change semantics. If however 
> dynamic initialization of global variable occurs in constant rounding mode 
> (changing FP control modes in initializers without restoring them is UB), 
> static and dynamic initialization would be semantically equivalent.
> 
> We cannot apply the same rule to local static variables, as they are treated 
> differently in C and C++. So the code:
> ```
> float func_01(float x) {
>   #pragma STDC FENV_ACCESS ON
>   static float val = 1.0/3.0;
>   return val;
> }
> ```
> Would be compiled differently in C and C++.
> 
Additional note:

If initialization is dynamic and constant rounding mode is not default, the 
body of initializer is executed with dynamic rounding mode set to the constant 
mode. That is, the code:
```
#pragma STDC FENV_ROUND FE_UPWARD
float var = some_init_expr;
```
generates code similar to:
```
float var = []()->float {
  #pragma STDC FENV_ROUND FE_UPWARD
  return some_init_expr;
}
```

So initializers of global variable must conform to:

1. If the initialization is dynamic and dynamic rounding mode is changed in the 
initializer, it must be restored upon finishing the initializer.
2. The initializer is evaluated in constant rounding mode. If the 
initialization is dynamic, the initializing code is executed in dynamic 
rounding mode set to the constant rounding mode.

These seems enough to provide compatibility with C and the same semantics for 
static and dynamic initialization.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88498

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

[PATCH] D86841: [clang] Add mustprogress and llvm.loop.mustprogress attribute deduction

[Atmn Patel via Phabricator via cfe-commits]

atmnpatel added inline comments.



Comment at: clang/lib/CodeGen/CGStmt.cpp:801
+ getLangOpts().CPlusPlus11 || getLangOpts().CPlusPlus14 ||
+ getLangOpts().CPlusPlus17 || getLangOpts().C2x) {
+LoopMustProgress = true;

aqjune wrote:
> A silly question: does old C/C++ not guarantee that loops should make forward 
> progress?
Nope, it was introduced explicitly simultaneously in C11 (6.8.5p6) and C++11 
has it implicitly through [intro.progress].


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86841

___
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 295469.
myhsu added a reviewer: aaron.ballman.
myhsu added a comment.

Fix formatting issues


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/M680x0.cpp
  clang/lib/Driver/ToolChains/Arch/M680x0.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/m680x0-features.cpp
  clang/test/Driver/m680x0-sub-archs.cpp

Index: clang/test/Driver/m680x0-sub-archs.cpp
===
--- /dev/null
+++ clang/test/Driver/m680x0-sub-archs.cpp
@@ -0,0 +1,23 @@
+// RUN: %clang -### -target m680x0-unknown-linux -mcpu=68000 %s 2>&1 | FileCheck --check-prefix=CHECK-MX00 %s
+// RUN: %clang -### -target m680x0-unknown-linux -m68000 %s 2>&1 | FileCheck --check-prefix=CHECK-MX00 %s
+// CHECK-MX00: "-target-cpu" "68000"
+
+// RUN: %clang -### -target m680x0-unknown-linux -mcpu=68010 %s 2>&1 | FileCheck --check-prefix=CHECK-MX10 %s
+// RUN: %clang -### -target m680x0-unknown-linux -m68010 %s 2>&1 | FileCheck --check-prefix=CHECK-MX10 %s
+// CHECK-MX10: "-target-cpu" "68010"
+
+// RUN: %clang -### -target m680x0-unknown-linux -mcpu=68020 %s 2>&1 | FileCheck --check-prefix=CHECK-MX20 %s
+// RUN: %clang -### -target m680x0-unknown-linux -m68020 %s 2>&1 | FileCheck --check-prefix=CHECK-MX20 %s
+// CHECK-MX20: "-target-cpu" "68020"
+
+// RUN: %clang -### -target m680x0-unknown-linux -mcpu=68030 %s 2>&1 | FileCheck --check-prefix=CHECK-MX30 %s
+// RUN: %clang -### -target m680x0-unknown-linux -m68030 %s 2>&1 | FileCheck --check-prefix=CHECK-MX30 %s
+// CHECK-MX30: "-target-cpu" "68030"
+
+// RUN: %clang -### -target m680x0-unknown-linux -mcpu=68040 %s 2>&1 | FileCheck --check-prefix=CHECK-MX40 %s
+// RUN: %clang -### -target m680x0-unknown-linux -m68040 %s 2>&1 | FileCheck --check-prefix=CHECK-MX40 %s
+// CHECK-MX40: "-target-cpu" "68040"
+
+// RUN: %clang -### -target m680x0-unknown-linux -mcpu=68060 %s 2>&1 | FileCheck --check-prefix=CHECK-MX60 %s
+// RUN: %clang -### -target m680x0-unknown-linux -m68060 %s 2>&1 | FileCheck --check-prefix=CHECK-MX60 %s
+// CHECK-MX60: "-target-cpu" "68060"
Index: clang/test/Driver/m680x0-features.cpp
===
--- /dev/null
+++ clang/test/Driver/m680x0-features.cpp
@@ -0,0 +1,45 @@
+// Check macro definitions
+// RUN: %clang -target m680x0-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 m680x0-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 m680x0-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 m680x0-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: #define mc68030 1
+
+// RUN: %clang -target m680x0-unknown-linux -m68040 -dM -E %s | FileCheck --check-prefix=CHECK-MX40 %s
+// CHECK-MX40: #define __mc68000 1
+// CHECK-MX40: #define __mc68000__ 1
+// CHECK-MX40: #define __mc68040 1
+// CHECK-MX40: #define __mc68040__ 1
+// CHECK-MX40: #define mc68000 1
+// CHECK-MX40: #define mc68040 1
+
+// RUN: %clang -target m680x0-unknown-linux -m68060 -dM -E %s | FileCheck --check-prefix=CHECK-MX60 %s
+// CHECK-MX60: #define __mc68000 1
+// CHECK-MX60: #define __mc68000__ 1
+// CHECK-MX60: #define __mc68060 1
+// CHECK-MX60: #define __mc68060__ 1
+// CHECK-MX60: #define mc68000 1
+// CHECK-MX60: #define mc68060 1
Index: clang/lib/Driver/ToolChains/Linux.cpp
===
--- clang/lib/Driver/ToolChains/Linux.cpp
+++ clang/lib/Driver/ToolChains/Linux.cpp
@@ -102,6 +102,12 @@
 if (D.getVFS().exists(SysRoot + "/lib/aarch64_be-linux-gnu"))
   return "aarch64_be-linux-gnu";
 break;
+
+  case llvm::Triple::m680x0:
+if (D.getVFS().exists(SysRoot + "/lib/m68k-linux-gnu"))
+  return "m68k-linux-gnu";
+break;
+
   case llvm::Triple::mips: {
 std::string MT = IsMipsR6 ? "mipsisa32r6-linux-gnu" : "mips-linux-gnu";
 if 

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

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

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

Fix formatting issues


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/M680x0.cpp
  clang/lib/Basic/Targets/M680x0.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
@@ -5797,6 +5797,39 @@
   D->addAttr(::new (S.Context) MipsInterruptAttr(S.Context, AL, Kind));
 }
 
+static void handleM680x0InterruptAttr(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) M680x0InterruptAttr(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.
@@ -6069,6 +6102,9 @@
   case llvm::Triple::mips:
 handleMipsInterruptAttr(S, D, AL);
 break;
+  case llvm::Triple::m680x0:
+handleM680x0InterruptAttr(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
@@ -8064,6 +8064,45 @@
   return false;
 }
 
+//===--===//
+// M680x0 ABI Implementation
+//===--===//
+
+namespace {
+
+class M680x0TargetCodeGenInfo : public TargetCodeGenInfo {
+public:
+  M680x0TargetCodeGenInfo(CodeGenTypes )
+  : TargetCodeGenInfo(std::make_unique(CGT)) {}
+  void setTargetAttributes(const Decl *D, llvm::GlobalValue *GV,
+   CodeGen::CodeGenModule ) const override;
+};
+
+} // namespace
+
+// TODO Does not actually work right now
+void M680x0TargetCodeGenInfo::setTargetAttributes(
+const Decl *D, llvm::GlobalValue *GV, CodeGen::CodeGenModule ) const {
+  if (const FunctionDecl *FD = dyn_cast_or_null(D)) {
+if (const M680x0InterruptAttr *attr = FD->getAttr()) {
+  // Handle 'interrupt' attribute:
+  llvm::Function *F = cast(GV);
+
+  // Step 1: Set ISR calling convention.
+  F->setCallingConv(llvm::CallingConv::M680x0_INTR);
+
+  // Step 2: Add attributes goodness.
+  F->addFnAttr(llvm::Attribute::NoInline);
+
+  // ??? is this right
+  // Step 3: Emit ISR vector alias.
+  unsigned Num = attr->getNumber() / 2;
+  llvm::GlobalAlias::create(llvm::Function::ExternalLinkage,
+"__isr_" + Twine(Num), F);
+}
+  }
+}
+
 //===--===//
 // AVR ABI Implementation.
 //===--===//
Index: clang/lib/Basic/Targets/M680x0.h
===
--- /dev/null
+++ clang/lib/Basic/Targets/M680x0.h
@@ -0,0 +1,58 @@
+//===--- M680x0.h - Declare M680x0 target feature support ---*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+// This file declares M680x0 TargetInfo objects.
+//
+//===--===//
+
+#ifndef M680X0_H_LTNCIPAD
+#define M680X0_H_LTNCIPAD
+
+#include "OSTargets.h"
+#include "clang/Basic/TargetInfo.h"
+#include "clang/Basic/TargetOptions.h"
+#include "llvm/ADT/Triple.h"
+#include "llvm/Support/Compiler.h"
+
+namespace clang {
+namespace targets {
+
+class 

[PATCH] D86671: [clang-tidy] Add new case type to check variables with Hungarian notation

[Douglas Chen via Phabricator via cfe-commits]

dougpuob added a comment.

In D86671#2304337 , @njames93 wrote:

> Not strictly relevant here, but this does open up the idea of enforcing the 
> style where an enum constant is prefixed by the initials of the enum name.

I like this idea. There is a case when `EnumConstantPrefix` and 
`EnumConstantCase=szHungarianNotation` options are set, it may take similar 
affect(which will be the first) or be overwritten? I can have it a try later.

Showing my conception as the following:

  // [Before]
  typedef enum {
  RevValid = -1,
  RevNone  = 0, 
  RevCrlReason = 1, 
  RevHold  = 2, 
  RevKeyCompromise = 3, 
  RevCaCompromise  = 4  
  } REVINFO_TYPE;
  
  // [After]
  typedef enum {
  rtRevValid = -1
  rtRevNone  = 0,
  rtRevCrlReason = 1,
  rtRevHold  = 2,
  rtRevKeyCompromise = 3,
  rtRevCaCompromise  = 4 
  } REVINFO_TYPE;
  
  // [After] EnumConstantPrefix first case
  // EnumConstantCase=snHungarianNotation
  // EnumConstantPrefix=pre_
  typedef enum {
  pre_rtRevValid = -1
  pre_rtRevNone  = 0,
  pre_rtRevCrlReason = 1,
  pre_rtRevHold  = 2,
  pre_rtRevKeyCompromise = 3,
  pre_rtRevCaCompromise  = 4 
  } REVINFO_TYPE;




Comment at: 
clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-hungarian-notation.cpp:25
+// RUN: {key: readability-identifier-naming.FunctionCase   , value: 
CamelCase },   \
+// RUN: {key: readability-identifier-naming.ClassCase  , value: 
szHungarianNotation }, \
+// RUN: {key: readability-identifier-naming.TypedefCase, value: 
szHungarianNotation }, \

njames93 wrote:
> Class names shouldn't use hungarian notation.
OK~ I have classified CheckOptions, and all test cases one by one in the next 
diff.

```
// RUN:   -config='{ CheckOptions: [ \
// RUN: { key: readability-identifier-naming.ClassMemberCase  , 
value: szHungarianNotation }, \
// RUN: { key: readability-identifier-naming.ConstantCase , 
value: szHungarianNotation }, \
// RUN: { key: readability-identifier-naming.ConstantMemberCase   , 
value: szHungarianNotation }, \
// RUN: { key: readability-identifier-naming.ConstantParameterCase, 
value: szHungarianNotation }, \
// RUN: { key: readability-identifier-naming.ConstantPointerParameterCase , 
value: szHungarianNotation }, \
// RUN: { key: readability-identifier-naming.ConstexprVariableCase, 
value: szHungarianNotation }, \
// RUN: { key: readability-identifier-naming.GlobalConstantCase   , 
value: szHungarianNotation }, \
// RUN: { key: readability-identifier-naming.GlobalConstantPointerCase, 
value: szHungarianNotation }, \
// RUN: { key: readability-identifier-naming.GlobalVariableCase   , 
value: szHungarianNotation }, \
// RUN: { key: readability-identifier-naming.LocalConstantCase, 
value: szHungarianNotation }, \
// RUN: { key: readability-identifier-naming.LocalConstantPointerCase , 
value: szHungarianNotation }, \
// RUN: { key: readability-identifier-naming.LocalPointerCase , 
value: szHungarianNotation }, \
// RUN: { key: readability-identifier-naming.LocalVariableCase, 
value: szHungarianNotation }, \
// RUN: { key: readability-identifier-naming.MemberCase   , 
value: szHungarianNotation }, \
// RUN: { key: readability-identifier-naming.ParameterCase, 
value: szHungarianNotation }, \
// RUN: { key: readability-identifier-naming.PointerParameterCase , 
value: szHungarianNotation }, \
// RUN: { key: readability-identifier-naming.PrivateMemberCase, 
value: szHungarianNotation }, \
// RUN: { key: readability-identifier-naming.StaticConstantCase   , 
value: szHungarianNotation }, \
// RUN: { key: readability-identifier-naming.StaticVariableCase   , 
value: szHungarianNotation }, \
// RUN: { key: readability-identifier-naming.StructCase   , 
value: szHungarianNotation }, \
// RUN: { key: readability-identifier-naming.UnionCase, 
value: szHungarianNotation }, \
// RUN: { key: readability-identifier-naming.VariableCase , 
value: szHungarianNotation }  \
// RUN:   ]}'
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86671

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

[PATCH] D88630: [clang/CMake] Respect LLVM_TOOLS_INSTALL_DIR

[Keno Fischer via Phabricator via cfe-commits]

loladiro created this revision.
loladiro added a reviewer: tstellar.
Herald added subscribers: cfe-commits, mgorny.
Herald added a project: clang.
loladiro requested review of this revision.

Otherwise clang installs all of its tools into `bin/` while
LLVM installs its tools into (LLVM_TOOLS_INSTALL_DIR).
I could swear this used to work (and in fact the julia build system
assumes it), but I can't pin down a specific commit that would
have broken this, and julia has been relying on pre-compiled binaries
for a while now (that don't use this setting), so it may have been
broken for quite a while.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D88630

Files:
  clang/cmake/modules/AddClang.cmake


Index: clang/cmake/modules/AddClang.cmake
===
--- clang/cmake/modules/AddClang.cmake
+++ clang/cmake/modules/AddClang.cmake
@@ -170,7 +170,7 @@
 
 install(TARGETS ${name}
   ${export_to_clangtargets}
-  RUNTIME DESTINATION bin
+  RUNTIME DESTINATION ${LLVM_TOOLS_INSTALL_DIR}
   COMPONENT ${name})
 
 if(NOT LLVM_ENABLE_IDE)


Index: clang/cmake/modules/AddClang.cmake
===
--- clang/cmake/modules/AddClang.cmake
+++ clang/cmake/modules/AddClang.cmake
@@ -170,7 +170,7 @@
 
 install(TARGETS ${name}
   ${export_to_clangtargets}
-  RUNTIME DESTINATION bin
+  RUNTIME DESTINATION ${LLVM_TOOLS_INSTALL_DIR}
   COMPONENT ${name})
 
 if(NOT LLVM_ENABLE_IDE)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D87974: [Builtin] Add __builtin_zero_non_value_bits.

[Zoe Carver via Phabricator via cfe-commits]

zoecarver added inline comments.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:1704
+->getArrayElementTypeNoTypeQual()
+->isRecordType()) {
+  auto FieldElement = CGF.Builder.CreateStructGEP(Ptr, Index);

Is it OK to possibly create hundreds of stores here? I assume later 
optimizations will catch this and turn it into a loop or a call to memset or 
something. But this could potentially be harmful to code size.


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] D87974: [Builtin] Add __builtin_zero_non_value_bits.

[Zoe Carver via Phabricator via cfe-commits]

zoecarver added inline comments.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:1652
+auto Element = CGF.Builder.CreateGEP(I8Ptr, Index);
+CGF.Builder.CreateAlignedStore(Zero, Element, MaybeAlign());
+  };

jfb wrote:
> You should use `alignmentAtOffset` here.
I'm using `CharUnits::One().alignmentAtOffset` to here because this type will 
always have a size of 1 because it's an `i8` ptr.


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] D87974: [Builtin] Add __builtin_zero_non_value_bits.

[Zoe Carver via Phabricator via cfe-commits]

zoecarver updated this revision to Diff 295445.
zoecarver added a comment.

- Support constant arrays
- Format changes with clang-format


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87974

Files:
  clang/include/clang/Basic/Builtins.def
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGenCXX/builtin-zero-non-value-bits-codegen.cpp
  clang/test/CodeGenCXX/builtin-zero-non-value-bits.cpp
  clang/test/SemaCXX/builtin-zero-non-value-bits.cpp

Index: clang/test/SemaCXX/builtin-zero-non-value-bits.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/builtin-zero-non-value-bits.cpp
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+struct Foo {};
+
+struct Incomplete; // expected-note {{forward declaration of 'Incomplete'}}
+
+void test(int a, Foo b, void *c, int *d, Foo *e, const Foo *f, Incomplete *g) {
+  __builtin_zero_non_value_bits(a); // expected-error {{passing 'int' to parameter of incompatible type structure pointer: type mismatch at 1st parameter ('int' vs structure pointer)}}
+  __builtin_zero_non_value_bits(b); // expected-error {{passing 'Foo' to parameter of incompatible type structure pointer: type mismatch at 1st parameter ('Foo' vs structure pointer)}}
+  __builtin_zero_non_value_bits(c); // expected-error {{passing 'void *' to parameter of incompatible type structure pointer: type mismatch at 1st parameter ('void *' vs structure pointer)}}
+  __builtin_zero_non_value_bits(d); // expected-error {{passing 'int *' to parameter of incompatible type structure pointer: type mismatch at 1st parameter ('int *' vs structure pointer)}}
+  __builtin_zero_non_value_bits(e); // This should not error.
+  __builtin_zero_non_value_bits(f); // expected-error {{read-only variable is not assignable}}
+  __builtin_zero_non_value_bits(g); // expected-error {{variable has incomplete type 'Incomplete'}}
+}
Index: clang/test/CodeGenCXX/builtin-zero-non-value-bits.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/builtin-zero-non-value-bits.cpp
@@ -0,0 +1,249 @@
+// RUN: mkdir -p %t
+// RUN: %clang++ %s -o %t/run
+// RUN: %t/run
+
+#include 
+#include 
+#include 
+#include 
+
+template 
+struct alignas(A1) BasicWithPadding {
+  T x;
+  alignas(A2) T y;
+};
+
+template 
+struct alignas(A1) SpacedArrayMembers {
+  T x[N];
+  alignas(A2) char c;
+  T y[N];
+};
+
+template 
+struct alignas(A1) PaddedPointerMembers {
+  T *x;
+  alignas(A2) T *y;
+};
+
+template 
+struct alignas(A1) ThreeMembers {
+  T x;
+  alignas(A2) T y;
+  alignas(A3) T z;
+};
+
+template 
+struct Normal {
+  T a;
+  T b;
+};
+
+template 
+struct X {
+  T x;
+};
+
+template 
+struct Z {
+  T z;
+};
+
+template 
+struct YZ : public Z {
+  alignas(A) T y;
+};
+
+template 
+struct alignas(A1) HasBase : public X, public YZ {
+  T a;
+  alignas(A2) T b;
+};
+
+template 
+void testAllForType(T a, T b, T c, T d) {
+  using B = BasicWithPadding;
+  B basic1;
+  memset(, 0, sizeof(B));
+  basic1.x = a;
+  basic1.y = b;
+  B basic2;
+  memset(, 42, sizeof(B));
+  basic2.x = a;
+  basic2.y = b;
+  assert(memcmp(, , sizeof(B)) != 0);
+  __builtin_zero_non_value_bits();
+  assert(memcmp(, , sizeof(B)) == 0);
+
+  using A = SpacedArrayMembers;
+  A arr1;
+  memset(, 0, sizeof(A));
+  arr1.x[0] = a;
+  arr1.x[1] = b;
+  arr1.y[0] = c;
+  arr1.y[1] = d;
+  A arr2;
+  memset(, 42, sizeof(A));
+  arr2.x[0] = a;
+  arr2.x[1] = b;
+  arr2.y[0] = c;
+  arr2.y[1] = d;
+  arr2.c = 0;
+  assert(memcmp(, , sizeof(A)) != 0);
+  __builtin_zero_non_value_bits();
+  assert(memcmp(, , sizeof(A)) == 0);
+
+  using P = PaddedPointerMembers;
+  P ptr1;
+  memset(, 0, sizeof(P));
+  ptr1.x = 
+  ptr1.y = 
+  P ptr2;
+  memset(, 42, sizeof(P));
+  ptr2.x = 
+  ptr2.y = 
+  assert(memcmp(, , sizeof(P)) != 0);
+  __builtin_zero_non_value_bits();
+  assert(memcmp(, , sizeof(P)) == 0);
+
+  using Three = ThreeMembers;
+  Three three1;
+  memset(, 0, sizeof(Three));
+  three1.x = a;
+  three1.y = b;
+  three1.z = c;
+  Three three2;
+  memset(, 42, sizeof(Three));
+  three2.x = a;
+  three2.y = b;
+  three2.z = c;
+  __builtin_zero_non_value_bits();
+  assert(memcmp(, , sizeof(Three)) == 0);
+
+  using N = Normal;
+  N normal1;
+  memset(, 0, sizeof(N));
+  normal1.a = a;
+  normal1.b = b;
+  N normal2;
+  memset(, 42, sizeof(N));
+  normal2.a = a;
+  normal2.b = b;
+  __builtin_zero_non_value_bits();
+  assert(memcmp(, , sizeof(N)) == 0);
+
+  using H = HasBase;
+  H base1;
+  memset(, 0, sizeof(H));
+  base1.a = a;
+  base1.b = b;
+  base1.x = c;
+  base1.y = d;
+  base1.z = a;
+  H base2;
+  memset(, 42, sizeof(H));
+  base2.a = a;
+  base2.b = b;
+  base2.x = c;
+  base2.y = d;
+  base2.z = a;
+  assert(memcmp(, , sizeof(H)) != 0);
+  __builtin_zero_non_value_bits();
+  unsigned i = 0;
+  assert(memcmp(, , sizeof(H)) == 0);
+}
+

[PATCH] D87974: [Builtin] Add __builtin_zero_non_value_bits.

[Zoe Carver via Phabricator via cfe-commits]

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



Comment at: clang/test/CodeGenCXX/builtin-zero-non-value-bits-codegen.cpp:46
+void test(Baz *baz) {
+  __builtin_zero_non_value_bits(baz);
+}

zoecarver wrote:
> jfb wrote:
> > It would be useful to see a test for arrays with a type that contains tail 
> > padding.
> Hmm, this test case doesn't seem to be working. I'll investigate further. 
OK, I've added that. Just to clarify, you mean a type that contains a constant 
array type of types with tail padding (i.e., `Bar [2]`)?


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

[clang] e4f50e5 - [ARM] Add missing target for Arm neon test case.

[Ranjeet Singh via cfe-commits]

Author: Ranjeet Singh
Date: 2020-10-01T00:32:33+01:00
New Revision: e4f50e587f077c246b7f29db0b7daddf583e2b64

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

LOG: [ARM] Add missing target for Arm neon test case.

This is a follow-up from https://reviews.llvm.org/D61717. Where Richard
described the issue with compiling arm_neon.h under
-flax-vector-conversions=none. It looks like the example reproducer does
actually work but what was missing was a test entry for that target.

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

Added: 


Modified: 
clang/test/Headers/arm-neon-header.c

Removed: 




diff  --git a/clang/test/Headers/arm-neon-header.c 
b/clang/test/Headers/arm-neon-header.c
index f6362886010a..8f64633b44d5 100644
--- a/clang/test/Headers/arm-neon-header.c
+++ b/clang/test/Headers/arm-neon-header.c
@@ -22,5 +22,6 @@
 
 // RUN: %clang -fsyntax-only -Wall -Werror -ffreestanding 
--target=aarch64-none-eabi -march=armv8.2-a+fp16fml+crypto+dotprod -std=c11 -xc 
-flax-vector-conversions=none %s
 // RUN: %clang -fsyntax-only -Wall -Werror -ffreestanding 
--target=aarch64_be-none-eabi -march=armv8.2-a+fp16fml+crypto+dotprod -std=c11 
-xc -flax-vector-conversions=none %s
+// RUN: %clang -fsyntax-only -Wall -Werror -ffreestanding 
--target=arm64-linux-gnu -arch +neon -std=c11 -xc -flax-vector-conversions=none 
%s
 
 #include 



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

[PATCH] D88546: [ARM] Update NEON testcase with missing target string in

[Ranjeet Singh via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGe4f50e587f07: [ARM] Add missing target for Arm neon test 
case. (authored by rs).

Changed prior to commit:
  https://reviews.llvm.org/D88546?vs=295219=295435#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88546

Files:
  clang/test/Headers/arm-neon-header.c


Index: clang/test/Headers/arm-neon-header.c
===
--- clang/test/Headers/arm-neon-header.c
+++ clang/test/Headers/arm-neon-header.c
@@ -22,5 +22,6 @@
 
 // RUN: %clang -fsyntax-only -Wall -Werror -ffreestanding 
--target=aarch64-none-eabi -march=armv8.2-a+fp16fml+crypto+dotprod -std=c11 -xc 
-flax-vector-conversions=none %s
 // RUN: %clang -fsyntax-only -Wall -Werror -ffreestanding 
--target=aarch64_be-none-eabi -march=armv8.2-a+fp16fml+crypto+dotprod -std=c11 
-xc -flax-vector-conversions=none %s
+// RUN: %clang -fsyntax-only -Wall -Werror -ffreestanding 
--target=arm64-linux-gnu -arch +neon -std=c11 -xc -flax-vector-conversions=none 
%s
 
 #include 


Index: clang/test/Headers/arm-neon-header.c
===
--- clang/test/Headers/arm-neon-header.c
+++ clang/test/Headers/arm-neon-header.c
@@ -22,5 +22,6 @@
 
 // RUN: %clang -fsyntax-only -Wall -Werror -ffreestanding --target=aarch64-none-eabi -march=armv8.2-a+fp16fml+crypto+dotprod -std=c11 -xc -flax-vector-conversions=none %s
 // RUN: %clang -fsyntax-only -Wall -Werror -ffreestanding --target=aarch64_be-none-eabi -march=armv8.2-a+fp16fml+crypto+dotprod -std=c11 -xc -flax-vector-conversions=none %s
+// RUN: %clang -fsyntax-only -Wall -Werror -ffreestanding --target=arm64-linux-gnu -arch +neon -std=c11 -xc -flax-vector-conversions=none %s
 
 #include 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 21cf2e6 - Handle unknown OSes in DarwinTargetInfo::getExnObjectAlignment

[Akira Hatanaka via cfe-commits]

Author: Akira Hatanaka
Date: 2020-09-30T16:05:17-07:00
New Revision: 21cf2e6c263d7a50654653bce4e83ab463fae580

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

LOG: Handle unknown OSes in DarwinTargetInfo::getExnObjectAlignment

rdar://problem/69727650

Added: 


Modified: 
clang/lib/Basic/Targets/OSTargets.h
clang/test/SemaCXX/warn-overaligned-type-thrown.cpp

Removed: 




diff  --git a/clang/lib/Basic/Targets/OSTargets.h 
b/clang/lib/Basic/Targets/OSTargets.h
index e07067693054..9b96690f413c 100644
--- a/clang/lib/Basic/Targets/OSTargets.h
+++ b/clang/lib/Basic/Targets/OSTargets.h
@@ -154,7 +154,8 @@ class LLVM_LIBRARY_VISIBILITY DarwinTargetInfo : public 
OSTargetInfo {
   MinVersion = llvm::VersionTuple(5U);
   break;
 default:
-  llvm_unreachable("Unexpected OS");
+  // Conservatively return 8 bytes if OS is unknown.
+  return 64;
 }
 
 unsigned Major, Minor, Micro;

diff  --git a/clang/test/SemaCXX/warn-overaligned-type-thrown.cpp 
b/clang/test/SemaCXX/warn-overaligned-type-thrown.cpp
index d7468445f8b7..9f2386ddc3c6 100644
--- a/clang/test/SemaCXX/warn-overaligned-type-thrown.cpp
+++ b/clang/test/SemaCXX/warn-overaligned-type-thrown.cpp
@@ -3,6 +3,7 @@
 // RUN: %clang_cc1 -triple arm64-apple-tvos10 -verify -fsyntax-only -std=c++11 
-fcxx-exceptions -fexceptions -DUNDERALIGNED %s
 // RUN: %clang_cc1 -triple arm64-apple-watchos4 -verify -fsyntax-only 
-std=c++11 -fcxx-exceptions -fexceptions -DUNDERALIGNED %s
 // RUN: %clang_cc1 -triple arm-linux-gnueabi -verify -fsyntax-only -std=c++11 
-fcxx-exceptions -fexceptions  -DUNDERALIGNED %s
+// RUN: %clang_cc1 -triple thumbv7em-apple-unknown-macho -verify -fsyntax-only 
-std=c++11 -fcxx-exceptions -fexceptions -DUNDERALIGNED %s
 // RUN: %clang_cc1 -triple x86_64-apple-macosx10.14 -verify -fsyntax-only 
-std=c++11 -fcxx-exceptions -fexceptions %s
 // RUN: %clang_cc1 -triple arm64-apple-ios12 -verify -fsyntax-only -std=c++11 
-fcxx-exceptions -fexceptions %s
 // RUN: %clang_cc1 -triple arm64-apple-tvos12 -verify -fsyntax-only -std=c++11 
-fcxx-exceptions -fexceptions %s



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

[PATCH] D86841: [clang] Add mustprogress and llvm.loop.mustprogress attribute deduction

[Juneyoung Lee via Phabricator via cfe-commits]

aqjune added inline comments.



Comment at: clang/lib/CodeGen/CGStmt.cpp:801
+ getLangOpts().CPlusPlus11 || getLangOpts().CPlusPlus14 ||
+ getLangOpts().CPlusPlus17 || getLangOpts().C2x) {
+LoopMustProgress = true;

A silly question: does old C/C++ not guarantee that loops should make forward 
progress?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86841

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

[PATCH] D88603: [WebAssembly] Add support for DWARF type units

[Derek Schuff via Phabricator via cfe-commits]

dschuff added inline comments.



Comment at: llvm/lib/MC/MCObjectFileInfo.cpp:962
+  case Triple::Wasm:
+return Ctx->getWasmSection(Name, SectionKind::getMetadata(), utostr(Hash),
+   ~0);

dschuff wrote:
> I may add a couple more tests to this, but I did want to ask @sbc100 about 
> this, since I'm not 100% sure at the uniqueID field is for.
also let me be more clear about the question here: what is `UniqueID` for, and 
is it bad that I'm just passing it a number that is totally not unique?



Comment at: llvm/lib/MC/WasmObjectWriter.cpp:1353
   if (Begin) {
 WasmIndices[cast(Begin)] = CustomSections.size();
-if (SectionName != Begin->getName())

sbc100 wrote:
> Just checking in the case where you were hitting this error the SectionName 
> was duplicate but the `Begin` is uniquified ?
> 
> 
right. There's a second .debug_info section, so the name of `Begin` gets 
uniquified.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88603

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

[PATCH] D71524: [analyzer] Support tainted objects in GenericTaintChecker

[Borsik Gábor via Phabricator via cfe-commits]

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

In D71524#2291925 , @steakhal wrote:

> In D71524#2284386 , @Szelethus wrote:
>
>> I figured you're still working on this, sorry! I'd really like to chat about 
>> my earlier comment D71524#1917251 , 
>> as it kind of challenges the high level idea.
>
> What about marking the `std::cin` object itself as tainted and any object 
> created by `ifstream::ifstream(const char*)` or similar functions.
> Then propagate taint via the extraction operator (`operator>>`) only if the 
> stream was tainted.
> This way we could reduce the false-positives of this crude heuristic. What do 
> you think?

As far as I remember I tried to make `std::cin` tainted, but it was 
complicated. I run the checker against many projects and there wasn't any false 
positive related to this heuristic.
We can restrict the `operator>>`  to `std::basic_stream` and cover only the 
standard library. I think most of the programmers will use this in a 
conventional way, therefore it should work for their implementation too.


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

https://reviews.llvm.org/D71524

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

[PATCH] D88477: [analyzer] Overwrite cast type in getBinding only if that was null originally

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

(in the latest message by "load" i mean the first load that produces a pointer 
(i.e., an `ElementRegion`) as the result)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88477

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

[clang-tools-extra] 85fc5bf - [clangd] Remove dead variable. NFC

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2020-09-30T23:19:15+02:00
New Revision: 85fc5bf341395171e67490061f6fbc76b297b78d

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

LOG: [clangd] Remove dead variable. NFC

Added: 


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

Removed: 




diff  --git a/clang-tools-extra/clangd/URI.cpp 
b/clang-tools-extra/clangd/URI.cpp
index f9e8fdc46fa7f..80e3a1017312d 100644
--- a/clang-tools-extra/clangd/URI.cpp
+++ b/clang-tools-extra/clangd/URI.cpp
@@ -111,7 +111,6 @@ bool shouldEscape(unsigned char C) {
 /// - Reserved characters always escaped with exceptions like '/'.
 /// - All other characters are escaped.
 void percentEncode(llvm::StringRef Content, std::string ) {
-  std::string Result;
   for (unsigned char C : Content)
 if (shouldEscape(C)) {
   Out.push_back('%');



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

[PATCH] D88477: [analyzer] Overwrite cast type in getBinding only if that was null originally

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

I'm trying to say that the value produced by the load should not be the same as 
the stored value, because these two values are of different types. When exactly 
does the first value change into the second value is a different story; the 
current grand vision around which the code is written says that it changes 
during load, therefore it's the load code (step #2) that's to blame.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88477

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

[clang] ae4c400 - [NFC] Fix spacing in clang/test/Driver/aix-ld.c

[Hubert Tong via cfe-commits]

Author: Hubert Tong
Date: 2020-09-30T17:01:32-04:00
New Revision: ae4c400e02fc3f7cff11cc332e6b107353b3e6a2

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

LOG: [NFC] Fix spacing in clang/test/Driver/aix-ld.c

Fix one line with mismatch in indentation after afc277b0ed0d.

Added: 


Modified: 
clang/test/Driver/aix-ld.c

Removed: 




diff  --git a/clang/test/Driver/aix-ld.c b/clang/test/Driver/aix-ld.c
index 7ccbeff3b8b6..89959d851b93 100644
--- a/clang/test/Driver/aix-ld.c
+++ b/clang/test/Driver/aix-ld.c
@@ -403,7 +403,7 @@
 // CHECK-LD32-NOSTDLIBXX-LCXX: "-L[[SYSROOT]]/usr/lib"
 // CHECK-LD32-NOSTDLIBXX-LCXX-NOT: "-lc++"
 // CHECK-LD32-NOSTDLIBXX-LCXX: 
"[[RESOURCE_DIR]]{{/|}}lib{{/|}}aix{{/|}}libclang_rt.builtins-powerpc.a"
-// CHECK-LD32-NOSTDLIBXX-LCXX:"-lm"
+// CHECK-LD32-NOSTDLIBXX-LCXX: "-lm"
 // CHECK-LD32-NOSTDLIBXX-LCXX: "-lc"
 
 // Check powerpc64-ibm-aix7.1.0.0, 64-bit. -nostdlib++.



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

[PATCH] D88138: [NPM] Add target specific hook to add passes for New Pass Manager

[Arthur Eubanks via Phabricator via cfe-commits]

aeubanks added a comment.

In D88138#2304243 , @quic_aankit wrote:

> @aeubanks Thanks for the review. Can you also commit the patch on my behalf?

Committed in ce5379f0f0675592fd10a522009fd5b1561ca72b 
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88138

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

[PATCH] D88138: [NPM] Add target specific hook to add passes for New Pass Manager

[Arthur Eubanks via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGce5379f0f067: [NPM] Add target specific hook to add passes 
for New Pass Manager (authored by aeubanks).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88138

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  llvm/include/llvm/Target/TargetMachine.h
  llvm/lib/Target/Hexagon/HexagonTargetMachine.cpp
  llvm/lib/Target/Hexagon/HexagonTargetMachine.h
  llvm/test/CodeGen/Hexagon/registerpassbuildercallbacks.ll
  llvm/tools/opt/NewPMDriver.cpp

Index: llvm/tools/opt/NewPMDriver.cpp
===
--- llvm/tools/opt/NewPMDriver.cpp
+++ llvm/tools/opt/NewPMDriver.cpp
@@ -375,6 +375,9 @@
   PB.registerLoopAnalyses(LAM);
   PB.crossRegisterProxies(LAM, FAM, CGAM, MAM);
 
+  if (TM)
+TM->registerPassBuilderCallbacks(PB, DebugPM);
+
   ModulePassManager MPM(DebugPM);
   if (VK > VK_NoVerifier)
 MPM.addPass(VerifierPass());
Index: llvm/test/CodeGen/Hexagon/registerpassbuildercallbacks.ll
===
--- /dev/null
+++ llvm/test/CodeGen/Hexagon/registerpassbuildercallbacks.ll
@@ -0,0 +1,27 @@
+; RUN: opt -mtriple=hexagon -disable-verify -debug-pass-manager \
+; RUN: -disable-output -passes='default' -S %s 2>&1 \
+; RUN: | FileCheck %s --check-prefix=NPM
+; RUN: opt -mtriple=hexagon -disable-verify -debug-pass-manager \
+; RUN: -disable-output -passes='default' -S %s 2>&1 \
+; RUN: | FileCheck %s --check-prefix=NPM
+; RUN: opt -mtriple=hexagon -disable-verify -debug-pass-manager \
+; RUN: -disable-output -passes='default' -S %s 2>&1 \
+; RUN: | FileCheck %s --check-prefix=NPM
+
+; Test TargetMachine::registerPassBuilderCallbacks
+; NPM: Running pass: HexagonVectorLoopCarriedReusePass
+
+declare void @bar() local_unnamed_addr
+
+define void @foo(i32 %n) local_unnamed_addr {
+entry:
+  br label %loop
+loop:
+  %iv = phi i32 [ 0, %entry ], [ %iv.next, %loop ]
+  %iv.next = add i32 %iv, 1
+  tail call void @bar()
+  %cmp = icmp eq i32 %iv, %n
+  br i1 %cmp, label %exit, label %loop
+exit:
+  ret void
+}
Index: llvm/lib/Target/Hexagon/HexagonTargetMachine.h
===
--- llvm/lib/Target/Hexagon/HexagonTargetMachine.h
+++ llvm/lib/Target/Hexagon/HexagonTargetMachine.h
@@ -37,6 +37,8 @@
   static unsigned getModuleMatchQuality(const Module );
 
   void adjustPassManager(PassManagerBuilder ) override;
+  void registerPassBuilderCallbacks(PassBuilder ,
+bool DebugPassManager) override;
   TargetPassConfig *createPassConfig(PassManagerBase ) override;
   TargetTransformInfo getTargetTransformInfo(const Function ) override;
 
Index: llvm/lib/Target/Hexagon/HexagonTargetMachine.cpp
===
--- llvm/lib/Target/Hexagon/HexagonTargetMachine.cpp
+++ llvm/lib/Target/Hexagon/HexagonTargetMachine.cpp
@@ -22,6 +22,7 @@
 #include "llvm/CodeGen/TargetPassConfig.h"
 #include "llvm/IR/LegacyPassManager.h"
 #include "llvm/IR/Module.h"
+#include "llvm/Passes/PassBuilder.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/TargetRegistry.h"
 #include "llvm/Transforms/IPO/PassManagerBuilder.h"
@@ -273,6 +274,18 @@
   });
 }
 
+void HexagonTargetMachine::registerPassBuilderCallbacks(PassBuilder ,
+bool DebugPassManager) {
+  PB.registerOptimizerLastEPCallback(
+  [=](ModulePassManager , PassBuilder::OptimizationLevel Level) {
+LoopPassManager LPM(DebugPassManager);
+FunctionPassManager FPM(DebugPassManager);
+LPM.addPass(HexagonVectorLoopCarriedReusePass());
+FPM.addPass(createFunctionToLoopPassAdaptor(std::move(LPM)));
+MPM.addPass(createModuleToFunctionPassAdaptor(std::move(FPM)));
+  });
+}
+
 TargetTransformInfo
 HexagonTargetMachine::getTargetTransformInfo(const Function ) {
   return TargetTransformInfo(HexagonTTIImpl(this, F));
Index: llvm/include/llvm/Target/TargetMachine.h
===
--- llvm/include/llvm/Target/TargetMachine.h
+++ llvm/include/llvm/Target/TargetMachine.h
@@ -34,6 +34,7 @@
 class MCSubtargetInfo;
 class MCSymbol;
 class raw_pwrite_stream;
+class PassBuilder;
 class PassManagerBuilder;
 struct PerFunctionMIParsingState;
 class SMDiagnostic;
@@ -294,6 +295,11 @@
   /// PassManagerBuilder::addExtension.
   virtual void adjustPassManager(PassManagerBuilder &) {}
 
+  /// Allow the target to modify the pass pipeline with New Pass Manager
+  /// (similar to adjustPassManager for Legacy Pass manager).
+  virtual void registerPassBuilderCallbacks(PassBuilder &,
+bool DebugPassManager) 

[clang] ce5379f - [NPM] Add target specific hook to add passes for New Pass Manager

[Arthur Eubanks via cfe-commits]

Author: Arthur Eubanks
Date: 2020-09-30T13:29:43-07:00
New Revision: ce5379f0f0675592fd10a522009fd5b1561ca72b

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

LOG: [NPM] Add target specific hook to add passes for New Pass Manager

The patch adds a new TargetMachine member "registerPassBuilderCallbacks" for 
targets to add passes to the pass pipeline using the New Pass Manager (similar 
to adjustPassManager for the Legacy Pass Manager).

Reviewed By: aeubanks

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

Added: 
llvm/test/CodeGen/Hexagon/registerpassbuildercallbacks.ll

Modified: 
clang/lib/CodeGen/BackendUtil.cpp
llvm/include/llvm/Target/TargetMachine.h
llvm/lib/Target/Hexagon/HexagonTargetMachine.cpp
llvm/lib/Target/Hexagon/HexagonTargetMachine.h
llvm/tools/opt/NewPMDriver.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/BackendUtil.cpp 
b/clang/lib/CodeGen/BackendUtil.cpp
index d77590cc2adf..dbd67a6ebe9b 100644
--- a/clang/lib/CodeGen/BackendUtil.cpp
+++ b/clang/lib/CodeGen/BackendUtil.cpp
@@ -1214,6 +1214,9 @@ void EmitAssemblyHelper::EmitAssemblyWithNewPassManager(
   PB.registerLoopAnalyses(LAM);
   PB.crossRegisterProxies(LAM, FAM, CGAM, MAM);
 
+  if (TM)
+TM->registerPassBuilderCallbacks(PB, CodeGenOpts.DebugPassManager);
+
   ModulePassManager MPM(CodeGenOpts.DebugPassManager);
 
   if (!CodeGenOpts.DisableLLVMPasses) {

diff  --git a/llvm/include/llvm/Target/TargetMachine.h 
b/llvm/include/llvm/Target/TargetMachine.h
index 60d4fb579bb9..4a9152832107 100644
--- a/llvm/include/llvm/Target/TargetMachine.h
+++ b/llvm/include/llvm/Target/TargetMachine.h
@@ -34,6 +34,7 @@ class MCRegisterInfo;
 class MCSubtargetInfo;
 class MCSymbol;
 class raw_pwrite_stream;
+class PassBuilder;
 class PassManagerBuilder;
 struct PerFunctionMIParsingState;
 class SMDiagnostic;
@@ -294,6 +295,11 @@ class TargetMachine {
   /// PassManagerBuilder::addExtension.
   virtual void adjustPassManager(PassManagerBuilder &) {}
 
+  /// Allow the target to modify the pass pipeline with New Pass Manager
+  /// (similar to adjustPassManager for Legacy Pass manager).
+  virtual void registerPassBuilderCallbacks(PassBuilder &,
+bool DebugPassManager) {}
+
   /// Add passes to the specified pass manager to get the specified file
   /// emitted.  Typically this will involve several steps of code generation.
   /// This method should return true if emission of this file type is not

diff  --git a/llvm/lib/Target/Hexagon/HexagonTargetMachine.cpp 
b/llvm/lib/Target/Hexagon/HexagonTargetMachine.cpp
index cb3b6fbdd69e..0f15c46bc8bb 100644
--- a/llvm/lib/Target/Hexagon/HexagonTargetMachine.cpp
+++ b/llvm/lib/Target/Hexagon/HexagonTargetMachine.cpp
@@ -22,6 +22,7 @@
 #include "llvm/CodeGen/TargetPassConfig.h"
 #include "llvm/IR/LegacyPassManager.h"
 #include "llvm/IR/Module.h"
+#include "llvm/Passes/PassBuilder.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/TargetRegistry.h"
 #include "llvm/Transforms/IPO/PassManagerBuilder.h"
@@ -273,6 +274,18 @@ void 
HexagonTargetMachine::adjustPassManager(PassManagerBuilder ) {
   });
 }
 
+void HexagonTargetMachine::registerPassBuilderCallbacks(PassBuilder ,
+bool DebugPassManager) 
{
+  PB.registerOptimizerLastEPCallback(
+  [=](ModulePassManager , PassBuilder::OptimizationLevel Level) {
+LoopPassManager LPM(DebugPassManager);
+FunctionPassManager FPM(DebugPassManager);
+LPM.addPass(HexagonVectorLoopCarriedReusePass());
+FPM.addPass(createFunctionToLoopPassAdaptor(std::move(LPM)));
+MPM.addPass(createModuleToFunctionPassAdaptor(std::move(FPM)));
+  });
+}
+
 TargetTransformInfo
 HexagonTargetMachine::getTargetTransformInfo(const Function ) {
   return TargetTransformInfo(HexagonTTIImpl(this, F));

diff  --git a/llvm/lib/Target/Hexagon/HexagonTargetMachine.h 
b/llvm/lib/Target/Hexagon/HexagonTargetMachine.h
index 7ee4474e90e3..fa174128f708 100644
--- a/llvm/lib/Target/Hexagon/HexagonTargetMachine.h
+++ b/llvm/lib/Target/Hexagon/HexagonTargetMachine.h
@@ -37,6 +37,8 @@ class HexagonTargetMachine : public LLVMTargetMachine {
   static unsigned getModuleMatchQuality(const Module );
 
   void adjustPassManager(PassManagerBuilder ) override;
+  void registerPassBuilderCallbacks(PassBuilder ,
+bool DebugPassManager) override;
   TargetPassConfig *createPassConfig(PassManagerBase ) override;
   TargetTransformInfo getTargetTransformInfo(const Function ) override;
 

diff  --git a/llvm/test/CodeGen/Hexagon/registerpassbuildercallbacks.ll 
b/llvm/test/CodeGen/Hexagon/registerpassbuildercallbacks.ll
new file mode 100644
index 

[PATCH] D86819: [PowerPC][Power10] Implementation of 128-bit Binary Vector Rotate builtins

[Albion Fung via Phabricator via cfe-commits]

Conanap updated this revision to Diff 295393.
Conanap marked 6 inline comments as done.
Conanap added a comment.

Addressed comments and corrected incorrect behaviour on Power10


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

https://reviews.llvm.org/D86819

Files:
  clang/include/clang/Basic/BuiltinsPPC.def
  clang/lib/Headers/altivec.h
  clang/test/CodeGen/builtins-ppc-p10vector.c
  llvm/include/llvm/IR/IntrinsicsPowerPC.td
  llvm/lib/Target/PowerPC/PPCISelLowering.cpp
  llvm/lib/Target/PowerPC/PPCInstrPrefix.td
  llvm/test/CodeGen/PowerPC/p10-vector-rotate.ll

Index: llvm/test/CodeGen/PowerPC/p10-vector-rotate.ll
===
--- /dev/null
+++ llvm/test/CodeGen/PowerPC/p10-vector-rotate.ll
@@ -0,0 +1,85 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64le-unknown-linux-gnu \
+; RUN:   -mcpu=pwr10 -ppc-asm-full-reg-names -ppc-vsr-nums-as-vr < %s | \
+; RUN:   FileCheck %s -check-prefixes=CHECK-LE,CHECK
+
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-linux-gnu \
+; RUN:   -mcpu=pwr10 -ppc-asm-full-reg-names -ppc-vsr-nums-as-vr < %s | \
+; RUN:   FileCheck %s -check-prefixes=CHECK-BE,CHECK
+
+; This test case aims to test the builtins for vector rotate instructions
+; on Power10.
+
+
+define <1 x i128> @test_vrlq(<1 x i128> %x, <1 x i128> %y) {
+; CHECK-LABEL: test_vrlq:
+; CHECK:   # %bb.0:
+; CHECK-NEXT:vrlq v2, v3, v2
+; CHECK-NEXT:blr
+  %shl.i = shl <1 x i128> %y, %x
+  %sub.i = sub <1 x i128> , %x
+  %lshr.i = lshr <1 x i128> %y, %sub.i
+  %tmp = or <1 x i128> %shl.i, %lshr.i
+  ret <1 x i128> %tmp
+}
+
+define <1 x i128> @test_vrlq_cost_mult8(<1 x i128> %x) {
+; CHECK-LABEL: test_vrlq_cost_mult8:
+; CHECK: # %bb.0:
+; CHECK: vrlq v2, v3, v2
+; CHECK-NEXT: blr
+  %shl.i = shl <1 x i128> , %x
+  %sub.i = sub <1 x i128> , %x
+  %lshr.i = lshr <1 x i128> , %sub.i
+  %tmp = or <1 x i128> %shl.i, %lshr.i
+  ret <1 x i128> %tmp
+}
+
+define <1 x i128> @test_vrlq_cost_non_mult8(<1 x i128> %x) {
+; CHECK-LABEL: test_vrlq_cost_non_mult8:
+; CHECK: # %bb.0:
+; CHECK: vrlq v2, v3, v2
+; CHECK-NEXT: blr
+  %shl.i = shl <1 x i128> , %x
+  %sub.i = sub <1 x i128> , %x
+  %lshr.i = lshr <1 x i128> , %sub.i
+  %tmp = or <1 x i128> %shl.i, %lshr.i
+  ret <1 x i128> %tmp
+}
+
+; Function Attrs: nounwind readnone
+define <1 x i128> @test_vrlqmi(<1 x i128> %a, <1 x i128> %b, <1 x i128> %c) {
+; CHECK-LABEL: test_vrlqmi:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:vrlqmi v3, v2, v4
+; CHECK-NEXT:vmr v2, v3
+; CHECK-NEXT:blr
+entry:
+  %tmp = tail call <1 x i128> @llvm.ppc.altivec.vrlqmi(<1 x i128> %a, <1 x i128> %c, <1 x i128> %b)
+  ret <1 x i128> %tmp
+}
+
+; Function Attrs: nounwind readnone
+define <1 x i128> @test_vrlqnm(<1 x i128> %a, <1 x i128> %b, <1 x i128> %c) {
+; CHECK-LABEL: test_vrlqnm:
+; CHECK:# %bb.0: # %entry
+; CHECK-BE: lxvx v5
+; CHECK-BE-NEXT:vperm v3, v3, v4, v5
+; CHECK-LE-NEXT:plxv v5
+; CHECK-LE-NEXT:vperm v3, v4, v3, v5
+; CHECK-NEXT:   vrlqnm v2, v2, v3
+; CHECK-NEXT:   blr
+entry:
+  %0 = bitcast <1 x i128> %b to <16 x i8>
+  %1 = bitcast <1 x i128> %c to <16 x i8>
+  %shuffle.i = shufflevector <16 x i8> %0, <16 x i8> %1, <16 x i32> 
+  %d = bitcast <16 x i8> %shuffle.i to <1 x i128>
+  %tmp = tail call <1 x i128> @llvm.ppc.altivec.vrlqnm(<1 x i128> %a, <1 x i128> %d)
+  ret <1 x i128> %tmp
+}
+
+; Function Attrs: nounwind readnone
+declare <1 x i128> @llvm.ppc.altivec.vrlqmi(<1 x i128>, <1 x i128>, <1 x i128>)
+
+; Function Attrs: nounwind readnone
+declare <1 x i128> @llvm.ppc.altivec.vrlqnm(<1 x i128>, <1 x i128>)
Index: llvm/lib/Target/PowerPC/PPCInstrPrefix.td
===
--- llvm/lib/Target/PowerPC/PPCInstrPrefix.td
+++ llvm/lib/Target/PowerPC/PPCInstrPrefix.td
@@ -1461,19 +1461,25 @@
"vcmpuq $BF, $vA, $vB", IIC_VecGeneral, []>;
   def VCMPSQ : VXForm_BF3_VAB5<321, (outs crrc:$BF), (ins vrrc:$vA, vrrc:$vB),
"vcmpsq $BF, $vA, $vB", IIC_VecGeneral, []>;
-  def VRLQNM : VX1_VT5_VA5_VB5<325, "vrlqnm", []>;
-  def VRLQMI : VXForm_1<69, (outs vrrc:$vD),
-(ins vrrc:$vA, vrrc:$vB, vrrc:$vDi),
-"vrlqmi $vD, $vA, $vB", IIC_VecFP, []>,
-RegConstraint<"$vDi = $vD">, NoEncode<"$vDi">;
   def VSLQ : VX1_VT5_VA5_VB5<261, "vslq", []>;
   def VSRAQ : VX1_VT5_VA5_VB5<773, "vsraq", []>;
   def VSRQ : VX1_VT5_VA5_VB5<517, "vsrq", []>;
-  def VRLQ : VX1_VT5_VA5_VB5<5, "vrlq", []>;
   def XSCVQPUQZ : X_VT5_XO5_VB5<63, 0, 836, "xscvqpuqz", []>;
   def XSCVQPSQZ : X_VT5_XO5_VB5<63, 8, 836, "xscvqpsqz", []>;
   def XSCVUQQP : X_VT5_XO5_VB5<63, 3, 836, "xscvuqqp", []>;
   def XSCVSQQP : X_VT5_XO5_VB5<63, 11, 836, "xscvsqqp", []>;
+  def VRLQ : VX1_VT5_VA5_VB5<5, "vrlq", []>;
+  def 

[PATCH] D52676: [clang-format] tweaked another case of lambda formatting

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added a comment.

I agree with both of you. I shouldn't have used the word "regression" indeed. I 
just meant a change in behaviour. Sorry for that.
I'll try to play around and propose a patch to enhance this feature :).
If you have any pointers about how to check if everything fits on a single line 
(it must be already done somewhere, nope?) then I'm all ears!


Repository:
  rL LLVM

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

https://reviews.llvm.org/D52676

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

[PATCH] D88477: [analyzer] Overwrite cast type in getBinding only if that was null originally

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

steakhal added a comment.

I'm getting lost :D

In D88477#2304230 , @NoQ wrote:

> And I believe that this part is already incorrect. Like, regardless of how we 
> do the dereference (the implicit lvalue-to-rvalue cast), or *whether* we do 
> it at all (nobody guarantees we'll ever do that!), the part of the static 
> analyzer that computes the lvalue `**b` has to work correctly. As of now it 
> computes an lvalue of incorrect type (currently it's `unsigned char` but it 
> has to be `char *`).

Are you implying that when we evaluate the assignment to the lvalue (line 
`#1`), we should have cast the stored value to the static type before binding 
in the `RegionStore`?

Doesn't it contradict with your previous statement:

In D77062#2298748 , @NoQ wrote:

> The contract of RegionStore with respect to type punning is that it stores 
> the value //as written//, even if its type doesn't match the storage type, 
> but then it casts the value to the correct type upon reading by invoking 
> `CastRetrievedVal()` on it. That's where the fix should probably be.



---

Or after evaluating the first dereference - but before evaluating the second - 
should we cast the lvalue to `char**` and only then do the LValueToRValue 
conversion?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88477

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

[PATCH] D88296: [clang-format] De-duplicate includes with leading or trailing whitespace.

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added a comment.

Thanks for the review.
I agree with you that a clang-format user should not expect a perfectly 
formatted code after a single iteration.
Maybe it's actually a documentation issue and we should be clear about it? Just 
a guarantee of 1) stability of formatting and 2) asymptotically arriving at the 
fully formatted code.
I'll think about how to word it and create a revision for the documentation 
(unless you judge it unnecessary).

BTW, I would be almost inclined to abandon this review and mark the bug report 
as invalid... but the change is so trivial...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88296

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

[PATCH] D86671: [clang-tidy] Add new case type to check variables with Hungarian notation

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

Not strictly relevant here, but this does open up the idea of enforcing the 
style where an enum constant is prefixed by the initials of the enum name.




Comment at: 
clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-hungarian-notation.cpp:25
+// RUN: {key: readability-identifier-naming.FunctionCase   , value: 
CamelCase },   \
+// RUN: {key: readability-identifier-naming.ClassCase  , value: 
szHungarianNotation }, \
+// RUN: {key: readability-identifier-naming.TypedefCase, value: 
szHungarianNotation }, \

Class names shouldn't use hungarian notation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86671

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

[clang] 1b60f63 - Revert "[OpenMP] Replace OpenMP RTL Functions With OMPIRBuilder and OMPKinds.def"

[Joseph Huber via cfe-commits]

Author: Joseph Huber
Date: 2020-09-30T15:12:21-04:00
New Revision: 1b60f63e4fd041550019b692dc7bf490dce2c75c

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

LOG: Revert "[OpenMP] Replace OpenMP RTL Functions With OMPIRBuilder and 
OMPKinds.def"

Failing tests on Arm due to the tests automatically populating
incomatible pointer width architectures. Reverting until the tests are
updated. Failing tests:

OpenMP/distribute_parallel_for_num_threads_codegen.cpp
OpenMP/distribute_parallel_for_if_codegen.cpp
OpenMP/distribute_parallel_for_simd_if_codegen.cpp
OpenMP/distribute_parallel_for_simd_num_threads_codegen.cpp
OpenMP/target_teams_distribute_parallel_for_if_codegen.cpp
OpenMP/target_teams_distribute_parallel_for_simd_if_codegen.cpp
OpenMP/teams_distribute_parallel_for_if_codegen.cpp
OpenMP/teams_distribute_parallel_for_simd_if_codegen.cpp

This reverts commit 90eaedda9b8ef46e2c0c1b8bce33e98a3adbb68c.

Added: 


Modified: 
clang/lib/CodeGen/CGOpenMPRuntime.h
clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
clang/lib/CodeGen/CodeGenModule.h
clang/test/OpenMP/nvptx_parallel_codegen.cpp
llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
llvm/test/Transforms/OpenMP/add_attributes.ll

Removed: 




diff  --git a/clang/lib/CodeGen/CGOpenMPRuntime.h 
b/clang/lib/CodeGen/CGOpenMPRuntime.h
index e39c2e11390e..41fa9f5345aa 100644
--- a/clang/lib/CodeGen/CGOpenMPRuntime.h
+++ b/clang/lib/CodeGen/CGOpenMPRuntime.h
@@ -306,9 +306,6 @@ class CGOpenMPRuntime {
   CodeGenModule 
   StringRef FirstSeparator, Separator;
 
-  /// An OpenMP-IR-Builder instance.
-  llvm::OpenMPIRBuilder OMPBuilder;
-
   /// Constructor allowing to redefine the name separator for the variables.
   explicit CGOpenMPRuntime(CodeGenModule , StringRef FirstSeparator,
StringRef Separator);
@@ -389,6 +386,8 @@ class CGOpenMPRuntime {
   llvm::Value *getCriticalRegionLock(StringRef CriticalName);
 
 private:
+  /// An OpenMP-IR-Builder instance.
+  llvm::OpenMPIRBuilder OMPBuilder;
 
   /// Map for SourceLocation and OpenMP runtime library debug locations.
   typedef llvm::DenseMap OpenMPDebugLocMapTy;

diff  --git a/clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp 
b/clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
index dbd24d33cc37..d9ef6c2a1078 100644
--- a/clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
+++ b/clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
@@ -28,6 +28,96 @@ using namespace CodeGen;
 using namespace llvm::omp;
 
 namespace {
+enum OpenMPRTLFunctionNVPTX {
+  /// Call to void __kmpc_kernel_init(kmp_int32 thread_limit,
+  /// int16_t RequiresOMPRuntime);
+  OMPRTL_NVPTX__kmpc_kernel_init,
+  /// Call to void __kmpc_kernel_deinit(int16_t IsOMPRuntimeInitialized);
+  OMPRTL_NVPTX__kmpc_kernel_deinit,
+  /// Call to void __kmpc_spmd_kernel_init(kmp_int32 thread_limit,
+  /// int16_t RequiresOMPRuntime, int16_t RequiresDataSharing);
+  OMPRTL_NVPTX__kmpc_spmd_kernel_init,
+  /// Call to void __kmpc_spmd_kernel_deinit_v2(int16_t RequiresOMPRuntime);
+  OMPRTL_NVPTX__kmpc_spmd_kernel_deinit_v2,
+  /// Call to void __kmpc_kernel_prepare_parallel(void
+  /// *outlined_function);
+  OMPRTL_NVPTX__kmpc_kernel_prepare_parallel,
+  /// Call to bool __kmpc_kernel_parallel(void **outlined_function);
+  OMPRTL_NVPTX__kmpc_kernel_parallel,
+  /// Call to void __kmpc_kernel_end_parallel();
+  OMPRTL_NVPTX__kmpc_kernel_end_parallel,
+  /// Call to void __kmpc_serialized_parallel(ident_t *loc, kmp_int32
+  /// global_tid);
+  OMPRTL_NVPTX__kmpc_serialized_parallel,
+  /// Call to void __kmpc_end_serialized_parallel(ident_t *loc, kmp_int32
+  /// global_tid);
+  OMPRTL_NVPTX__kmpc_end_serialized_parallel,
+  /// Call to int32_t __kmpc_shuffle_int32(int32_t element,
+  /// int16_t lane_offset, int16_t warp_size);
+  OMPRTL_NVPTX__kmpc_shuffle_int32,
+  /// Call to int64_t __kmpc_shuffle_int64(int64_t element,
+  /// int16_t lane_offset, int16_t warp_size);
+  OMPRTL_NVPTX__kmpc_shuffle_int64,
+  /// Call to __kmpc_nvptx_parallel_reduce_nowait_v2(ident_t *loc, kmp_int32
+  /// global_tid, kmp_int32 num_vars, size_t reduce_size, void* reduce_data,
+  /// void (*kmp_ShuffleReductFctPtr)(void *rhsData, int16_t lane_id, int16_t
+  /// lane_offset, int16_t shortCircuit),
+  /// void (*kmp_InterWarpCopyFctPtr)(void* src, int32_t warp_num));
+  OMPRTL_NVPTX__kmpc_nvptx_parallel_reduce_nowait_v2,
+  /// Call to __kmpc_nvptx_teams_reduce_nowait_v2(ident_t *loc, kmp_int32
+  /// global_tid, void *global_buffer, int32_t num_of_records, void*
+  /// reduce_data,
+  /// void (*kmp_ShuffleReductFctPtr)(void *rhsData, int16_t lane_id, int16_t
+  /// lane_offset, int16_t shortCircuit),
+  /// void (*kmp_InterWarpCopyFctPtr)(void* src, int32_t warp_num), void
+  /// (*kmp_ListToGlobalCpyFctPtr)(void *buffer, int idx, void *reduce_data),

[clang] 81921eb - [CodeGen] improve coverage for float (32-bit) type of NAN; NFC

[Sanjay Patel via cfe-commits]

Author: Sanjay Patel
Date: 2020-09-30T15:10:25-04:00
New Revision: 81921ebc430536ae5718da70a54328c790c8ae19

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

LOG: [CodeGen] improve coverage for float (32-bit) type of NAN; NFC

Goes with D88238

Added: 


Modified: 
clang/test/CodeGen/builtin-nan-exception.c

Removed: 




diff  --git a/clang/test/CodeGen/builtin-nan-exception.c 
b/clang/test/CodeGen/builtin-nan-exception.c
index 2acf0c4390ec..a0de25e52ebe 100644
--- a/clang/test/CodeGen/builtin-nan-exception.c
+++ b/clang/test/CodeGen/builtin-nan-exception.c
@@ -5,18 +5,28 @@
 
 // Run a variety of targets to ensure there's no target-based 
diff erence.
 
-// The builtin always produces a 64-bit (double).
 // An SNaN with no payload is formed by setting the bit after the
 // the quiet bit (MSB of the significand).
 
 // CHECK: float 0x7FF8, float 0x7FF4
-// CHECK: double 0x7FF8, double 0x7FF4
 
 float f[] = {
+  __builtin_nanf(""),
+  __builtin_nansf(""),
+};
+
+
+// Doubles are created and converted to floats.
+
+// CHECK: float 0x7FF8, float 0x7FF4
+
+float converted_to_float[] = {
   __builtin_nan(""),
   __builtin_nans(""),
 };
 
+// CHECK: double 0x7FF8, double 0x7FF4
+
 double d[] = {
   __builtin_nan(""),
   __builtin_nans(""),



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

[clang] bdc8529 - Revert "[OpenMP] Add Error Handling for Conflicting Pointer Sizes for Target Offload"

[via cfe-commits]

Author: Joseph Huber
Date: 2020-09-30T15:08:22-04:00
New Revision: bdc85292fb0f2a3965c8c65f9461d285b04841ed

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

LOG: Revert "[OpenMP] Add Error Handling for Conflicting Pointer Sizes for 
Target Offload"

Failing tests on Arm due to the tests automatically populating
incomatible pointer width architectures. Reverting until the tests are
updated. Failing tests:

OpenMP/distribute_parallel_for_num_threads_codegen.cpp
OpenMP/distribute_parallel_for_if_codegen.cpp
OpenMP/distribute_parallel_for_simd_if_codegen.cpp
OpenMP/distribute_parallel_for_simd_num_threads_codegen.cpp
OpenMP/target_teams_distribute_parallel_for_if_codegen.cpp
OpenMP/target_teams_distribute_parallel_for_simd_if_codegen.cpp
OpenMP/teams_distribute_parallel_for_if_codegen.cpp
OpenMP/teams_distribute_parallel_for_simd_if_codegen.cpp

This reverts commit 9d2378b59150f6f1cb5c9cf42ea06b0bb57029a1.

Added: 


Modified: 
clang/include/clang/Basic/DiagnosticDriverKinds.td
clang/lib/Frontend/CompilerInvocation.cpp
clang/test/OpenMP/nvptx_target_parallel_reduction_codegen_tbaa_PR46146.cpp

Removed: 
clang/test/OpenMP/target_incompatible_architecture_messages.cpp



diff  --git a/clang/include/clang/Basic/DiagnosticDriverKinds.td 
b/clang/include/clang/Basic/DiagnosticDriverKinds.td
index 29bc19e5a84e..3bf1bb19b7ae 100644
--- a/clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ b/clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -253,7 +253,6 @@ def err_drv_optimization_remark_format : Error<
   "unknown remark serializer format: '%0'">;
 def err_drv_no_neon_modifier : Error<"[no]neon is not accepted as modifier, 
please use [no]simd instead">;
 def err_drv_invalid_omp_target : Error<"OpenMP target is invalid: '%0'">;
-def err_drv_incompatible_omp_arch : Error<"OpenMP target architecture '%0' 
pointer size is incompatible with host '%1'">;
 def err_drv_omp_host_ir_file_not_found : Error<
   "The provided host compiler IR file '%0' is required to generate code for 
OpenMP target regions but cannot be found.">;
 def err_drv_omp_host_target_not_supported : Error<

diff  --git a/clang/lib/Frontend/CompilerInvocation.cpp 
b/clang/lib/Frontend/CompilerInvocation.cpp
index bbdf0e3be7ae..b402f53cc765 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -3206,14 +3206,6 @@ static void ParseLangArgs(LangOptions , ArgList 
, InputKind IK,
 TT.getArch() == llvm::Triple::x86 ||
 TT.getArch() == llvm::Triple::x86_64))
 Diags.Report(diag::err_drv_invalid_omp_target) << A->getValue(i);
-  else if ((T.isArch64Bit() && TT.isArch32Bit()) ||
-   (T.isArch64Bit() && TT.isArch16Bit()) ||
-   (T.isArch32Bit() && TT.isArch64Bit()) ||
-   (T.isArch32Bit() && TT.isArch16Bit()) ||
-   (T.isArch16Bit() && TT.isArch32Bit()) ||
-   (T.isArch16Bit() && TT.isArch64Bit()))
-Diags.Report(diag::err_drv_incompatible_omp_arch)
-<< A->getValue(i) << T.str();
   else
 Opts.OMPTargetTriples.push_back(TT);
 }

diff  --git 
a/clang/test/OpenMP/nvptx_target_parallel_reduction_codegen_tbaa_PR46146.cpp 
b/clang/test/OpenMP/nvptx_target_parallel_reduction_codegen_tbaa_PR46146.cpp
index 031c7b6c778e..aefe00f1cadf 100644
--- a/clang/test/OpenMP/nvptx_target_parallel_reduction_codegen_tbaa_PR46146.cpp
+++ b/clang/test/OpenMP/nvptx_target_parallel_reduction_codegen_tbaa_PR46146.cpp
@@ -1,8 +1,8 @@
 // RUN: %clang_cc1 -x c++ -O1 -disable-llvm-optzns -verify -fopenmp 
-internal-isystem %S/../Headers/Inputs/include -internal-isystem 
%S/../../lib/Headers/openmp_wrappers -include __clang_openmp_device_functions.h 
-triple powerpc64le-unknown-unknown -fopenmp-targets=nvptx64-nvidia-cuda 
-emit-llvm-bc %s -o %t-ppc-host.bc
 // RUN: %clang_cc1 -x c++ -O1 -disable-llvm-optzns -verify -fopenmp 
-internal-isystem %S/../Headers/Inputs/include -internal-isystem 
%S/../../lib/Headers/openmp_wrappers -include __clang_openmp_device_functions.h 
-triple nvptx64-unknown-unknown -aux-triple powerpc64le-unknown-unknown  
-fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm %s -fopenmp-is-device 
-fopenmp-host-ir-file-path %t-ppc-host.bc -o - | FileCheck %s
 // RUN: %clang_cc1 -x c++ -O1 -disable-llvm-optzns -verify -fopenmp 
-internal-isystem %S/../Headers/Inputs/include -internal-isystem 
%S/../../lib/Headers/openmp_wrappers -include __clang_openmp_device_functions.h 
-triple i386-unknown-unknown -fopenmp-targets=nvptx-nvidia-cuda -emit-llvm-bc 
%s -o %t-x86-host.bc
-// RUN: %clang_cc1 -x c++ -O1 -disable-llvm-optzns -verify -fopenmp 
-internal-isystem %S/../Headers/Inputs/include -internal-isystem 
%S/../../lib/Headers/openmp_wrappers -include 

[PATCH] D88603: [WebAssembly] Add support for DWARF type units

[Sam Clegg via Phabricator via cfe-commits]

sbc100 accepted this revision.
sbc100 added inline comments.
This revision is now accepted and ready to land.



Comment at: llvm/lib/MC/WasmObjectWriter.cpp:1353
   if (Begin) {
 WasmIndices[cast(Begin)] = CustomSections.size();
-if (SectionName != Begin->getName())

Just checking in the case where you were hitting this error the SectionName was 
duplicate but the `Begin` is uniquified ?




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88603

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

[PATCH] D88603: [WebAssembly] Add support for DWARF type units

[Derek Schuff via Phabricator via cfe-commits]

dschuff added inline comments.



Comment at: llvm/lib/MC/MCObjectFileInfo.cpp:962
+  case Triple::Wasm:
+return Ctx->getWasmSection(Name, SectionKind::getMetadata(), utostr(Hash),
+   ~0);

I may add a couple more tests to this, but I did want to ask @sbc100 about 
this, since I'm not 100% sure at the uniqueID field is for.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88603

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

[PATCH] D88603: [WebAssembly] Add support for DWARF type units

[Derek Schuff via Phabricator via cfe-commits]

dschuff created this revision.
dschuff added reviewers: aardappel, sbc100.
Herald added subscribers: llvm-commits, cfe-commits, ecnelises, sunfish, 
hiraditya, jgravelle-google.
Herald added projects: clang, LLVM.
dschuff requested review of this revision.
Herald added subscribers: ormris, aheejin.

Since Wasm comdat sections work similarly to ELF, we can use that mechanism
to eliminate duplicate dwarf type information in the same way.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D88603

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/debug-options.c
  llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
  llvm/lib/MC/MCObjectFileInfo.cpp
  llvm/lib/MC/WasmObjectWriter.cpp
  llvm/test/DebugInfo/WebAssembly/dwarf-headers.ll

Index: llvm/test/DebugInfo/WebAssembly/dwarf-headers.ll
===
--- /dev/null
+++ llvm/test/DebugInfo/WebAssembly/dwarf-headers.ll
@@ -0,0 +1,116 @@
+; RUN: llc -dwarf-version=4 -generate-type-units \
+; RUN: -filetype=obj -O0 -mtriple=wasm32-unknown-unknown < %s \
+; RUN: | llvm-dwarfdump -v - | FileCheck %s --check-prefix=SINGLE-4
+
+; RUN: llc -split-dwarf-file=foo.dwo -split-dwarf-output=%t.dwo \
+; RUN: -dwarf-version=4 -generate-type-units \
+; RUN: -filetype=obj -O0 -mtriple=wasm32-unknown-unknown < %s \
+; RUN: | llvm-dwarfdump -v - | FileCheck %s --check-prefix=O-4
+; RUN: llvm-dwarfdump -v %t.dwo | FileCheck %s --check-prefix=DWO-4
+
+; TODO: enable testing for dwarf v5
+; RUN: llc -dwarf-version=5 -generate-type-units \
+; RUN: -filetype=obj -O0 -mtriple= < %s \
+; RUN: | llvm-dwarfdump -v - | FileCheck %s --check-prefix=SINGLE-5
+
+; RUN: llc -split-dwarf-file=foo.dwo -split-dwarf-output=%t.dwo \
+; RUN: -dwarf-version=5 -generate-type-units \
+; RUN: -filetype=obj -O0 -mtriple= < %s \
+; RUN: | llvm-dwarfdump -v - | FileCheck %s --check-prefix=O-5
+; RUN: llvm-dwarfdump -v %t.dwo | FileCheck %s --check-prefix=DWO-5
+
+; This test is derived from test/CodeGen/X86/dwarf-headers.ll
+
+; Looking for DWARF headers to be generated correctly.
+; There are 8 variants with 5 formats: v4 CU, v4 TU, v5 normal/partial CU,
+; v5 skeleton/split CU, v5 normal/split TU.  Some v5 variants differ only
+; in the unit_type code, and the skeleton/split CU differs from normal/partial
+; by having one extra field (dwo_id).
+; (v2 thru v4 CUs are all the same, and TUs were invented in v4,
+; so we don't bother checking older versions.)
+
+; Test case built from:
+;struct S {
+;  int s1;
+;};
+;
+;S s;
+
+; Verify the v4 non-split headers.
+; Note that we check the exact offset of the DIEs because that tells us
+; the length of the header.
+;
+; SINGLE-4: .debug_info contents:
+; SINGLE-4: 0x: Compile Unit: {{.*}} version = 0x0004, abbr_offset
+; SINGLE-4: 0x000b: DW_TAG_compile_unit
+;
+; SINGLE-4: .debug_types contents:
+; SINGLE-4: 0x: Type Unit: {{.*}} version = 0x0004, abbr_offset
+; SINGLE-4: 0x0017: DW_TAG_type_unit
+
+; Verify the v4 split headers.
+;
+; O-4: .debug_info contents:
+; O-4: 0x: Compile Unit: {{.*}} version = 0x0004, abbr_offset
+; O-4: 0x000b: DW_TAG_compile_unit
+;
+; DWO-4: .debug_info.dwo contents:
+; DWO-4: 0x: Compile Unit: {{.*}} version = 0x0004, abbr_offset
+; DWO-4: 0x000b: DW_TAG_compile_unit
+;
+; DWO-4: .debug_types.dwo contents:
+; DWO-4: 0x: Type Unit: {{.*}} version = 0x0004, abbr_offset
+; DWO-4: 0x0017: DW_TAG_type_unit
+
+; Verify the v5 non-split headers. Type units come first.
+; All .debug_info sections are reported in one go, but the offset resets for
+; each new section.
+;
+; SINGLE-5: .debug_info contents:
+; SINGLE-5: 0x: Type Unit: {{.*}} version = 0x0005, unit_type = DW_UT_type, abbr_offset
+; SINGLE-5: 0x0018: DW_TAG_type_unit
+; SINGLE-5-NOT: contents:
+; SINGLE-5: 0x: Compile Unit: {{.*}} version = 0x0005, unit_type = DW_UT_compile, abbr_offset
+; SINGLE-5: 0x000c: DW_TAG_compile_unit
+
+; Verify the v5 split headers.
+;
+; O-5: .debug_info contents:
+; O-5: 0x: Compile Unit: {{.*}} version = 0x0005, unit_type = DW_UT_skeleton, abbr_offset
+; O-5-SAME:DWO_id = 0xccd7e58ef8bf4aa6
+; O-5: 0x0014: DW_TAG_skeleton_unit 
+;
+; DWO-5: .debug_info.dwo contents:
+; DWO-5: 0x: Type Unit: {{.*}} version = 0x0005, unit_type = DW_UT_split_type, abbr_offset
+; DWO-5: 0x0018: DW_TAG_type_unit
+; DWO-5: 0x0033: Compile Unit: {{.*}} version = 0x0005, unit_type = DW_UT_split_compile, abbr_offset
+; DWO-5-SAME:DWO_id = 0xccd7e58ef8bf4aa6
+; DWO-5: 0x0047: DW_TAG_compile_unit
+
+
+; ModuleID = 't.cpp'
+source_filename = "t.cpp"
+target datalayout = "e-m:e-p:32:32-i64:64-n32:64-S128"
+target triple = "wasm32-unknown-unknown-wasm"
+
+%struct.S = type { i32 }
+
+@s = global %struct.S zeroinitializer, align 4, !dbg !0
+
+!llvm.dbg.cu = !{!2}
+!llvm.module.flags = !{!10, !11}
+!llvm.ident = !{!12}
+
+!0 = 

[PATCH] D88524: [CUDA][HIP] Fix bound arch for offload action for fat binary

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

In D88524#2304224 , @yaxunl wrote:

> This translates to "unknown" in string form, which feels arbitrary. What if a 
> target has a valid cpu name which "unknown"? Isn't a nullptr (empty string in 
> string form) a more generic format to represent an invalid target?

It was the `nullptr` that got me digging. I was not sure what it would mean and 
do. Generally speaking it's a 'magic' value with the meaning that's not at all 
clear, even if it does what you want. I'd rather have a constant or enum with a 
clear meaning.

> Is it OK to make the change HIP specific? i.e. let HIP toolchain use empty 
> string for invalid target whereas CUDA toolchain keep using "unknown".

Putting magic value behind if(HIP) does not make it any better.

`TargetID(nullptr)` is the same as `TargetID("")` because StringRef(nullptr) is 
the same as an empty string. I'd add a CudaArch::UNUSED and modify 
`CudaVersionToString` to return "".  That should make 
`HIPToolChain::TranslateArgs()`skip it as it would for nullptr.


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

https://reviews.llvm.org/D88524

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

[PATCH] D88536: [clangd] Split DecisionForest Evaluate() into one func per tree.

[Utkarsh Saxena via Phabricator via cfe-commits]

usaxena95 added inline comments.



Comment at: clang-tools-extra/clangd/quality/CompletionModelCodegen.py:168
+nline.join(class_members),
+nline.join(["  float EvaluateTree%d() const;" % tree_num
+for tree_num in range(num_trees)]),

adamcz wrote:
> Why are these member functions? Why not keep them in .cc file only, in 
> anonymous namespace?
> 
> I wonder if that will make compiler inline them and then bring back msan 
> issues though.
> Why are these member functions? Why not keep them in .cc file only, in 
> anonymous namespace?

We will need to add getters for member variables (hoping they would be 
inlined). 
SG ?
(Or make the members public o_O ?)

> I wonder if that will make compiler inline them and then bring back MSAN 
> issues though.
 
I think we can disable that using  `LLVM_ATTRIBUTE_NOINLINE`



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88536

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

[PATCH] D88524: [CUDA][HIP] Fix bound arch for offload action for fat binary

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

In D88524#2304173 , @tra wrote:

>> Currently CUDA/HIP toolchain uses "unknown" as bound arch
>> for offload action for fat binary. This causes -mcpu or -march
>> with "unknown" added in HIPToolChain::TranslateArgs or
>> CUDAToolChain::TranslateArgs.
>
> It would appear that the problem is actually where we check TargetID -- we 
> should've ignored CudaArch::UNKNOWN there.
> Not setting the arch here avoids triggering the bug but it does not fix it.
> Considering that `CudaArch::UNKNOWN` is used here to indicate that the arch 
> is unused, perhaps we need an enum for that to distinguish it from 
> unknown/unset.

This translates to "unknown" in string form, which feels arbitrary. What if a 
target has a valid cpu name which "unknown"? Isn't a nullptr (empty string in 
string form) a more generic format to represent an invalid target?

Is it OK to make the change HIP specific? i.e. let HIP toolchain use empty 
string for invalid target whereas CUDA toolchain keep using "unknown".


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

https://reviews.llvm.org/D88524

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

[PATCH] D88477: [analyzer] Overwrite cast type in getBinding only if that was null originally

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

In D88477#2303376 , @steakhal wrote:

> In the Summary's example, at location `#2` the value of `**p` is 
> `{SymRegion{reg_$0},0 S64b,unsigned char}` - which is 
> exactly what we stored at line `#1`.
>
>   void test(int *a, char ***b, float *c) {
> *(unsigned char **)b = (unsigned char *)a; // #1
> if (**b == 0) // no-crash, #2
>   ;
> // ...
>   }

And I believe that this part is already incorrect. Like, regardless of how we 
do the dereference (the implicit lvalue-to-rvalue cast), or *whether* we do it 
at all (nobody guarantees we'll ever do that!), the part of the static analyzer 
that computes the lvalue `**b` has to work correctly. As of now it computes an 
lvalue of incorrect type (currently it's `unsigned char` but it has to be `char 
*`).

In D88477#2303376 , @steakhal wrote:

> IMO, we should relax this invariant in the following way:
> If the region is typed and the type also explicitly specified, we will 
> perform a cast to the explicitly stated type.
> In other words, overwrite the LoadTy only if that was not given.

This would teach the modeling of the load step for the pointee how to undo the 
damage done by the previous load step for the pointer. In the general case the 
load step for the pointee does not necessarily exist, therefore we cannot rely 
on it to undo the damage. This is why it's extremely important for every 
modeling step to be correct. You can't miss a detail on one step and account 
for it in another step because there's almost never a guarantee that the other 
step will actually have a chance to kick in.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88477

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

[PATCH] D88594: [OpenMP] Add Error Handling for Conflicting Pointer Sizes for Target Offload

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 closed this revision.
jhuber6 added a comment.

Committed in rG9d2378b59150 
, forgot 
to update the commit message.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88594

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

[PATCH] D88524: [CUDA][HIP] Fix bound arch for offload action for fat binary

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

> Currently CUDA/HIP toolchain uses "unknown" as bound arch
> for offload action for fat binary. This causes -mcpu or -march
> with "unknown" added in HIPToolChain::TranslateArgs or
> CUDAToolChain::TranslateArgs.

It would appear that the problem is actually where we check TargetID -- we 
should've ignored CudaArch::UNKNOWN there.
Not setting the arch here avoids triggering the bug but it does not fix it.
Considering that `CudaArch::UNKNOWN` is used here to indicate that the arch is 
unused, perhaps we need an enum for that to distinguish it from unknown/unset.


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

https://reviews.llvm.org/D88524

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

[PATCH] D88500: [AIX][Clang][Driver] Link libm in c++ mode

[David Tenty via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGafc277b0ed0d: [AIX][Clang][Driver] Link libm in c++ mode 
(authored by daltenty).

Changed prior to commit:
  https://reviews.llvm.org/D88500?vs=295111=295355#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88500

Files:
  clang/lib/Driver/ToolChains/AIX.cpp
  clang/test/Driver/aix-ld.c

Index: clang/test/Driver/aix-ld.c
===
--- clang/test/Driver/aix-ld.c
+++ clang/test/Driver/aix-ld.c
@@ -20,6 +20,7 @@
 // CHECK-LD32: "-L[[SYSROOT]]/usr/lib"
 // CHECK-LD32-NOT: "-lc++"
 // CHECK-LD32: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}aix{{/|}}libclang_rt.builtins-powerpc.a"
+// CHECK-LD32-NOT: "-lm"
 // CHECK-LD32: "-lc"
 
 // Check powerpc64-ibm-aix7.1.0.0, 64-bit.
@@ -41,6 +42,7 @@
 // CHECK-LD64: "-L[[SYSROOT]]/usr/lib"
 // CHECK-LD64-NOT: "-lc++"
 // CHECK-LD64: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}aix{{/|}}libclang_rt.builtins-powerpc64.a"
+// CHECK-LD64-NOT: "-lm"
 // CHECK-LD64: "-lc"
 
 // Check powerpc-ibm-aix7.1.0.0, 32-bit. Enable POSIX thread support.
@@ -64,6 +66,7 @@
 // CHECK-LD32-PTHREAD-NOT: "-lc++"
 // CHECK-LD32-PTHREAD: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}aix{{/|}}libclang_rt.builtins-powerpc.a"
 // CHECK-LD32-PTHREAD: "-lpthreads"
+// CHECK-LD32-PTHREAD-NOT: "-lm"
 // CHECK-LD32-PTHREAD: "-lc"
 
 // Check powerpc64-ibm-aix7.1.0.0, 64-bit. POSIX thread alias.
@@ -87,6 +90,7 @@
 // CHECK-LD64-PTHREAD-NOT: "-lc++"
 // CHECK-LD64-PTHREAD: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}aix{{/|}}libclang_rt.builtins-powerpc64.a"
 // CHECK-LD64-PTHREAD: "-lpthreads"
+// CHECK-LD64-PTHREAD-NOT: "-lm"
 // CHECK-LD64-PTHREAD: "-lc"
 
 // Check powerpc-ibm-aix7.1.0.0, 32-bit. Enable profiling.
@@ -109,6 +113,7 @@
 // CHECK-LD32-PROF: "-L[[SYSROOT]]/usr/lib"
 // CHECK-LD32-PROF-NOT: "-lc++"
 // CHECK-LD32-PROF: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}aix{{/|}}libclang_rt.builtins-powerpc.a"
+// CHECK-LD32-PROF-NOT: "-lm"
 // CHECK-LD32-PROF: "-lc"
 
 // Check powerpc64-ibm-aix7.1.0.0, 64-bit. Enable g-profiling.
@@ -131,6 +136,7 @@
 // CHECK-LD64-GPROF: "-L[[SYSROOT]]/usr/lib"
 // CHECK-LD64-GPROF-NOT: "-lc++"
 // CHECK-LD64-GPROF: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}aix{{/|}}libclang_rt.builtins-powerpc64.a"
+// CHECK-LD64-GPROF-NOT: "-lm"
 // CHECK-LD64-GPROF: "-lc"
 
 // Check powerpc-ibm-aix7.1.0.0, 32-bit. Static linking.
@@ -153,6 +159,7 @@
 // CHECK-LD32-STATIC: "-L[[SYSROOT]]/usr/lib"
 // CHECK-LD32-STATIC-NOT: "-lc++"
 // CHECK-LD32-STATIC: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}aix{{/|}}libclang_rt.builtins-powerpc.a"
+// CHECK-LD32-STATIC-NOT: "-lm"
 // CHECK-LD32-STATIC: "-lc"
 
 // Check powerpc-ibm-aix7.1.0.0, 32-bit. Library search path.
@@ -176,6 +183,7 @@
 // CHECK-LD32-LIBP: "-L[[SYSROOT]]/usr/lib"
 // CHECK-LD32-LIBP-NOT: "-lc++"
 // CHECK-LD32-LIBP: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}aix{{/|}}libclang_rt.builtins-powerpc.a"
+// CHECK-LD32-LIBP-NOT: "-lm"
 // CHECK-LD32-LIBP: "-lc"
 
 // Check powerpc-ibm-aix7.1.0.0, 32-bit. nostdlib.
@@ -200,6 +208,7 @@
 // CHECK-LD32-NO-STD-LIB-NOT: "-lc++"
 // CHECK-LD32-NO-STD-LIB-NOT: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}aix{{/|}}libclang_rt.builtins-powerpc.a"
 // CHECK-LD32-NO-STD-LIB-NOT: "-lpthreads"
+// CHECK-LD32-NO-STD-LIB-NOT: "-lm"
 // CHECK-LD32-NO-STD-LIB-NOT: "-lc"
 
 // Check powerpc64-ibm-aix7.1.0.0, 64-bit. nodefaultlibs.
@@ -224,6 +233,7 @@
 // CHECK-LD64-NO-DEFAULT-LIBS-NOT: "-lc++"
 // CHECK-LD64-NO-DEFAULT-LIBS-NOT: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}aix{{/|}}libclang_rt.builtins-powerpc64.a"
 // CHECK-LD64-NO-DEFAULT-LIBS-NOT: "-lpthreads"
+// CHECK-LD64-NO-DEFAULT-LIBS-NOT: "-lm"
 // CHECK-LD64-NO-DEFAULT-LIBS-NOT: "-lc"
 
 // Check powerpc-ibm-aix7.1.0.0, 32-bit. 'bcdtors' and argument order.
@@ -247,6 +257,7 @@
 // CHECK-LD32-CXX-ARG-ORDER-NOT: "-bcdtors:all:0:s"
 // CHECK-LD32-CXX-ARG-ORDER: "-lc++"
 // CHECK-LD32-CXX-ARG-ORDER: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}aix{{/|}}libclang_rt.builtins-powerpc.a"
+// CHECK-LD32-CXX-ARG-ORDER: "-lm"
 // CHECK-LD32-CXX-ARG-ORDER: "-lc"
 
 // Check powerpc-ibm-aix7.1.0.0, 32-bit. lc++ and lc order.
@@ -266,6 +277,7 @@
 // CHECK-LD32-CXX-ARG-LCXX: "-L[[SYSROOT]]/usr/lib"
 // CHECK-LD32-CXX-ARG-LCXX: "-lc++"
 // CHECK-LD32-CXX-ARG-LCXX: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}aix{{/|}}libclang_rt.builtins-powerpc.a"
+// CHECK-LD32-CXX-ARG-LCXX: "-lm"
 // CHECK-LD32-CXX-ARG-LCXX: "-lc"
 
 // Check powerpc64-ibm-aix7.1.0.0, 64-bit. lc++ and lc order.
@@ -285,6 +297,7 @@
 // CHECK-LD64-CXX-ARG-LCXX: "-L[[SYSROOT]]/usr/lib"
 // CHECK-LD64-CXX-ARG-LCXX: "-lc++"
 // CHECK-LD64-CXX-ARG-LCXX: 

[clang] afc277b - [AIX][Clang][Driver] Link libm in c++ mode

[David Tenty via cfe-commits]

Author: David Tenty
Date: 2020-09-30T14:02:17-04:00
New Revision: afc277b0ed0dcd9fbbde6015bbdf289349fb2104

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

LOG: [AIX][Clang][Driver] Link libm in c++ mode

since that is the normal behaviour of other compilers on the platform.

Reviewed By: hubert.reinterpretcast

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

Added: 


Modified: 
clang/lib/Driver/ToolChains/AIX.cpp
clang/test/Driver/aix-ld.c

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/AIX.cpp 
b/clang/lib/Driver/ToolChains/AIX.cpp
index b56ddf55cb30..351b34e8bf90 100644
--- a/clang/lib/Driver/ToolChains/AIX.cpp
+++ b/clang/lib/Driver/ToolChains/AIX.cpp
@@ -162,6 +162,9 @@ void aix::Linker::ConstructJob(Compilation , const 
JobAction ,
 if (Args.hasArg(options::OPT_pthreads, options::OPT_pthread))
   CmdArgs.push_back("-lpthreads");
 
+if (D.CCCIsCXX())
+  CmdArgs.push_back("-lm");
+
 CmdArgs.push_back("-lc");
   }
 

diff  --git a/clang/test/Driver/aix-ld.c b/clang/test/Driver/aix-ld.c
index 224e355aac13..7ccbeff3b8b6 100644
--- a/clang/test/Driver/aix-ld.c
+++ b/clang/test/Driver/aix-ld.c
@@ -20,6 +20,7 @@
 // CHECK-LD32: "-L[[SYSROOT]]/usr/lib"
 // CHECK-LD32-NOT: "-lc++"
 // CHECK-LD32: 
"[[RESOURCE_DIR]]{{/|}}lib{{/|}}aix{{/|}}libclang_rt.builtins-powerpc.a"
+// CHECK-LD32-NOT: "-lm"
 // CHECK-LD32: "-lc"
 
 // Check powerpc64-ibm-aix7.1.0.0, 64-bit.
@@ -41,6 +42,7 @@
 // CHECK-LD64: "-L[[SYSROOT]]/usr/lib"
 // CHECK-LD64-NOT: "-lc++"
 // CHECK-LD64: 
"[[RESOURCE_DIR]]{{/|}}lib{{/|}}aix{{/|}}libclang_rt.builtins-powerpc64.a"
+// CHECK-LD64-NOT: "-lm"
 // CHECK-LD64: "-lc"
 
 // Check powerpc-ibm-aix7.1.0.0, 32-bit. Enable POSIX thread support.
@@ -64,6 +66,7 @@
 // CHECK-LD32-PTHREAD-NOT: "-lc++"
 // CHECK-LD32-PTHREAD: 
"[[RESOURCE_DIR]]{{/|}}lib{{/|}}aix{{/|}}libclang_rt.builtins-powerpc.a"
 // CHECK-LD32-PTHREAD: "-lpthreads"
+// CHECK-LD32-PTHREAD-NOT: "-lm"
 // CHECK-LD32-PTHREAD: "-lc"
 
 // Check powerpc64-ibm-aix7.1.0.0, 64-bit. POSIX thread alias.
@@ -87,6 +90,7 @@
 // CHECK-LD64-PTHREAD-NOT: "-lc++"
 // CHECK-LD64-PTHREAD: 
"[[RESOURCE_DIR]]{{/|}}lib{{/|}}aix{{/|}}libclang_rt.builtins-powerpc64.a"
 // CHECK-LD64-PTHREAD: "-lpthreads"
+// CHECK-LD64-PTHREAD-NOT: "-lm"
 // CHECK-LD64-PTHREAD: "-lc"
 
 // Check powerpc-ibm-aix7.1.0.0, 32-bit. Enable profiling.
@@ -109,6 +113,7 @@
 // CHECK-LD32-PROF: "-L[[SYSROOT]]/usr/lib"
 // CHECK-LD32-PROF-NOT: "-lc++"
 // CHECK-LD32-PROF: 
"[[RESOURCE_DIR]]{{/|}}lib{{/|}}aix{{/|}}libclang_rt.builtins-powerpc.a"
+// CHECK-LD32-PROF-NOT: "-lm"
 // CHECK-LD32-PROF: "-lc"
 
 // Check powerpc64-ibm-aix7.1.0.0, 64-bit. Enable g-profiling.
@@ -131,6 +136,7 @@
 // CHECK-LD64-GPROF: "-L[[SYSROOT]]/usr/lib"
 // CHECK-LD64-GPROF-NOT: "-lc++"
 // CHECK-LD64-GPROF: 
"[[RESOURCE_DIR]]{{/|}}lib{{/|}}aix{{/|}}libclang_rt.builtins-powerpc64.a"
+// CHECK-LD64-GPROF-NOT: "-lm"
 // CHECK-LD64-GPROF: "-lc"
 
 // Check powerpc-ibm-aix7.1.0.0, 32-bit. Static linking.
@@ -153,6 +159,7 @@
 // CHECK-LD32-STATIC: "-L[[SYSROOT]]/usr/lib"
 // CHECK-LD32-STATIC-NOT: "-lc++"
 // CHECK-LD32-STATIC: 
"[[RESOURCE_DIR]]{{/|}}lib{{/|}}aix{{/|}}libclang_rt.builtins-powerpc.a"
+// CHECK-LD32-STATIC-NOT: "-lm"
 // CHECK-LD32-STATIC: "-lc"
 
 // Check powerpc-ibm-aix7.1.0.0, 32-bit. Library search path.
@@ -176,6 +183,7 @@
 // CHECK-LD32-LIBP: "-L[[SYSROOT]]/usr/lib"
 // CHECK-LD32-LIBP-NOT: "-lc++"
 // CHECK-LD32-LIBP: 
"[[RESOURCE_DIR]]{{/|}}lib{{/|}}aix{{/|}}libclang_rt.builtins-powerpc.a"
+// CHECK-LD32-LIBP-NOT: "-lm"
 // CHECK-LD32-LIBP: "-lc"
 
 // Check powerpc-ibm-aix7.1.0.0, 32-bit. nostdlib.
@@ -200,6 +208,7 @@
 // CHECK-LD32-NO-STD-LIB-NOT: "-lc++"
 // CHECK-LD32-NO-STD-LIB-NOT: 
"[[RESOURCE_DIR]]{{/|}}lib{{/|}}aix{{/|}}libclang_rt.builtins-powerpc.a"
 // CHECK-LD32-NO-STD-LIB-NOT: "-lpthreads"
+// CHECK-LD32-NO-STD-LIB-NOT: "-lm"
 // CHECK-LD32-NO-STD-LIB-NOT: "-lc"
 
 // Check powerpc64-ibm-aix7.1.0.0, 64-bit. nodefaultlibs.
@@ -224,6 +233,7 @@
 // CHECK-LD64-NO-DEFAULT-LIBS-NOT: "-lc++"
 // CHECK-LD64-NO-DEFAULT-LIBS-NOT: 
"[[RESOURCE_DIR]]{{/|}}lib{{/|}}aix{{/|}}libclang_rt.builtins-powerpc64.a"
 // CHECK-LD64-NO-DEFAULT-LIBS-NOT: "-lpthreads"
+// CHECK-LD64-NO-DEFAULT-LIBS-NOT: "-lm"
 // CHECK-LD64-NO-DEFAULT-LIBS-NOT: "-lc"
 
 // Check powerpc-ibm-aix7.1.0.0, 32-bit. 'bcdtors' and argument order.
@@ -247,6 +257,7 @@
 // CHECK-LD32-CXX-ARG-ORDER-NOT: "-bcdtors:all:0:s"
 // CHECK-LD32-CXX-ARG-ORDER: "-lc++"
 // CHECK-LD32-CXX-ARG-ORDER: 

[PATCH] D88430: [OpenMP] Replace OpenMP RTL Functions With OMPIRBuilder and OMPKinds.def

[Joseph Huber via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG90eaedda9b8e: [OpenMP] Replace OpenMP RTL Functions With 
OMPIRBuilder and OMPKinds.def (authored by jhuber6).

Changed prior to commit:
  https://reviews.llvm.org/D88430?vs=295343=295353#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88430

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.h
  clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/test/OpenMP/nvptx_parallel_codegen.cpp
  llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
  llvm/test/Transforms/OpenMP/add_attributes.ll

Index: llvm/test/Transforms/OpenMP/add_attributes.ll
===
--- llvm/test/Transforms/OpenMP/add_attributes.ll
+++ llvm/test/Transforms/OpenMP/add_attributes.ll
@@ -888,313 +888,313 @@
 ; CHECK: declare dso_local i32 @omp_pause_resource_all(i32)
 
 ; CHECK: ; Function Attrs: nounwind
-; CHECK-NEXT: declare dso_local i32 @omp_get_supported_active_levels() #0
+; CHECK-NEXT: declare dso_local i32 @omp_get_supported_active_levels()
 
-; CHECK: ; Function Attrs: nounwind
-; CHECK-NEXT: declare void @__kmpc_barrier(%struct.ident_t*, i32) #0
+; CHECK: ; Function Attrs: convergent nounwind
+; CHECK-NEXT: declare void @__kmpc_barrier(%struct.ident_t*, i32)
 
 ; CHECK: ; Function Attrs: nounwind
-; CHECK-NEXT: declare i32 @__kmpc_cancel(%struct.ident_t*, i32, i32) #0
+; CHECK-NEXT: declare i32 @__kmpc_cancel(%struct.ident_t*, i32, i32)
 
-; CHECK: ; Function Attrs: nounwind
-; CHECK-NEXT: declare i32 @__kmpc_cancel_barrier(%struct.ident_t*, i32) #0
+; CHECK: ; Function Attrs: convergent nounwind
+; CHECK-NEXT: declare i32 @__kmpc_cancel_barrier(%struct.ident_t*, i32)
 
-; CHECK: ; Function Attrs: nounwind
-; CHECK-NEXT: declare void @__kmpc_flush(%struct.ident_t*) #0
+; CHECK: ; Function Attrs: convergent nounwind
+; CHECK-NEXT: declare void @__kmpc_flush(%struct.ident_t*)
 
 ; CHECK: ; Function Attrs: nounwind
-; CHECK-NEXT: declare i32 @__kmpc_global_thread_num(%struct.ident_t*) #0
+; CHECK-NEXT: declare i32 @__kmpc_global_thread_num(%struct.ident_t*)
 
 ; CHECK: ; Function Attrs: nounwind
-; CHECK-NEXT: declare void @__kmpc_fork_call(%struct.ident_t*, i32, void (i32*, i32*, ...)*, ...) #0
+; CHECK-NEXT: declare void @__kmpc_fork_call(%struct.ident_t*, i32, void (i32*, i32*, ...)*, ...)
 
-; CHECK: ; Function Attrs: nounwind
-; CHECK-NEXT: declare i32 @__kmpc_omp_taskwait(%struct.ident_t*, i32) #0
+; CHECK: ; Function Attrs: convergent nounwind
+; CHECK-NEXT: declare i32 @__kmpc_omp_taskwait(%struct.ident_t*, i32)
 
 ; CHECK: ; Function Attrs: nounwind
-; CHECK-NEXT: declare i32 @__kmpc_omp_taskyield(%struct.ident_t*, i32, i32) #0
+; CHECK-NEXT: declare i32 @__kmpc_omp_taskyield(%struct.ident_t*, i32, i32)
 
 ; CHECK: ; Function Attrs: nounwind
-; CHECK-NEXT: declare void @__kmpc_push_num_threads(%struct.ident_t*, i32, i32) #0
+; CHECK-NEXT: declare void @__kmpc_push_num_threads(%struct.ident_t*, i32, i32)
 
 ; CHECK: ; Function Attrs: nounwind
-; CHECK-NEXT: declare void @__kmpc_push_proc_bind(%struct.ident_t*, i32, i32) #0
+; CHECK-NEXT: declare void @__kmpc_push_proc_bind(%struct.ident_t*, i32, i32)
 
 ; CHECK: ; Function Attrs: nounwind
-; CHECK-NEXT: declare void @__kmpc_serialized_parallel(%struct.ident_t*, i32) #0
+; CHECK-NEXT: declare void @__kmpc_serialized_parallel(%struct.ident_t*, i32)
 
 ; CHECK: ; Function Attrs: nounwind
-; CHECK-NEXT: declare void @__kmpc_end_serialized_parallel(%struct.ident_t*, i32) #0
+; CHECK-NEXT: declare void @__kmpc_end_serialized_parallel(%struct.ident_t*, i32)
 
 ; CHECK: ; Function Attrs: nounwind
-; CHECK-NEXT: declare i32 @__kmpc_master(%struct.ident_t*, i32) #0
+; CHECK-NEXT: declare i32 @__kmpc_master(%struct.ident_t*, i32)
 
 ; CHECK: ; Function Attrs: nounwind
-; CHECK-NEXT: declare void @__kmpc_end_master(%struct.ident_t*, i32) #0
+; CHECK-NEXT: declare void @__kmpc_end_master(%struct.ident_t*, i32)
 
-; CHECK: ; Function Attrs: nounwind
-; CHECK-NEXT: declare void @__kmpc_critical(%struct.ident_t*, i32, [8 x i32]*) #0
+; CHECK: ; Function Attrs: convergent nounwind
+; CHECK-NEXT: declare void @__kmpc_critical(%struct.ident_t*, i32, [8 x i32]*)
 
-; CHECK: ; Function Attrs: nounwind
-; CHECK-NEXT: declare void @__kmpc_critical_with_hint(%struct.ident_t*, i32, [8 x i32]*, i32) #0
+; CHECK: ; Function Attrs: convergent nounwind
+; CHECK-NEXT: declare void @__kmpc_critical_with_hint(%struct.ident_t*, i32, [8 x i32]*, i32)
 
-; CHECK: ; Function Attrs: nounwind
-; CHECK-NEXT: declare void @__kmpc_end_critical(%struct.ident_t*, i32, [8 x i32]*) #0
+; CHECK: ; Function Attrs: convergent nounwind
+; CHECK-NEXT: declare void @__kmpc_end_critical(%struct.ident_t*, i32, [8 x i32]*)
 
 ; CHECK: ; Function Attrs: nounwind
-; CHECK-NEXT: declare void @__kmpc_begin(%struct.ident_t*, i32) #0
+; CHECK-NEXT: declare void 

[clang] 90eaedd - [OpenMP] Replace OpenMP RTL Functions With OMPIRBuilder and OMPKinds.def

[Joseph Huber via cfe-commits]

Author: Joseph Huber
Date: 2020-09-30T14:00:01-04:00
New Revision: 90eaedda9b8ef46e2c0c1b8bce33e98a3adbb68c

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

LOG: [OpenMP] Replace OpenMP RTL Functions With OMPIRBuilder and OMPKinds.def

Summary:
Replace the OpenMP Runtime Library functions used in CGOpenMPRuntimeGPU
for OpenMP device code generation with ones in OMPKinds.def and use
OMPIRBuilder for generating runtime calls. This allows us to consolidate
more OpenMP code generation into the OMPIRBuilder. This patch also
invalidates specifying target architectures with conflicting pointer
sizes.

Reviewers: jdoerfert

Subscribers: aaron.ballman cfe-commits guansong llvm-commits sstefan1 yaxunl

Tags: #OpenMP #Clang #LLVM

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

Added: 


Modified: 
clang/lib/CodeGen/CGOpenMPRuntime.h
clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
clang/lib/CodeGen/CodeGenModule.h
clang/test/OpenMP/nvptx_parallel_codegen.cpp
llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
llvm/test/Transforms/OpenMP/add_attributes.ll

Removed: 




diff  --git a/clang/lib/CodeGen/CGOpenMPRuntime.h 
b/clang/lib/CodeGen/CGOpenMPRuntime.h
index 41fa9f5345aa..e39c2e11390e 100644
--- a/clang/lib/CodeGen/CGOpenMPRuntime.h
+++ b/clang/lib/CodeGen/CGOpenMPRuntime.h
@@ -306,6 +306,9 @@ class CGOpenMPRuntime {
   CodeGenModule 
   StringRef FirstSeparator, Separator;
 
+  /// An OpenMP-IR-Builder instance.
+  llvm::OpenMPIRBuilder OMPBuilder;
+
   /// Constructor allowing to redefine the name separator for the variables.
   explicit CGOpenMPRuntime(CodeGenModule , StringRef FirstSeparator,
StringRef Separator);
@@ -386,8 +389,6 @@ class CGOpenMPRuntime {
   llvm::Value *getCriticalRegionLock(StringRef CriticalName);
 
 private:
-  /// An OpenMP-IR-Builder instance.
-  llvm::OpenMPIRBuilder OMPBuilder;
 
   /// Map for SourceLocation and OpenMP runtime library debug locations.
   typedef llvm::DenseMap OpenMPDebugLocMapTy;

diff  --git a/clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp 
b/clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
index d9ef6c2a1078..dbd24d33cc37 100644
--- a/clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
+++ b/clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
@@ -28,96 +28,6 @@ using namespace CodeGen;
 using namespace llvm::omp;
 
 namespace {
-enum OpenMPRTLFunctionNVPTX {
-  /// Call to void __kmpc_kernel_init(kmp_int32 thread_limit,
-  /// int16_t RequiresOMPRuntime);
-  OMPRTL_NVPTX__kmpc_kernel_init,
-  /// Call to void __kmpc_kernel_deinit(int16_t IsOMPRuntimeInitialized);
-  OMPRTL_NVPTX__kmpc_kernel_deinit,
-  /// Call to void __kmpc_spmd_kernel_init(kmp_int32 thread_limit,
-  /// int16_t RequiresOMPRuntime, int16_t RequiresDataSharing);
-  OMPRTL_NVPTX__kmpc_spmd_kernel_init,
-  /// Call to void __kmpc_spmd_kernel_deinit_v2(int16_t RequiresOMPRuntime);
-  OMPRTL_NVPTX__kmpc_spmd_kernel_deinit_v2,
-  /// Call to void __kmpc_kernel_prepare_parallel(void
-  /// *outlined_function);
-  OMPRTL_NVPTX__kmpc_kernel_prepare_parallel,
-  /// Call to bool __kmpc_kernel_parallel(void **outlined_function);
-  OMPRTL_NVPTX__kmpc_kernel_parallel,
-  /// Call to void __kmpc_kernel_end_parallel();
-  OMPRTL_NVPTX__kmpc_kernel_end_parallel,
-  /// Call to void __kmpc_serialized_parallel(ident_t *loc, kmp_int32
-  /// global_tid);
-  OMPRTL_NVPTX__kmpc_serialized_parallel,
-  /// Call to void __kmpc_end_serialized_parallel(ident_t *loc, kmp_int32
-  /// global_tid);
-  OMPRTL_NVPTX__kmpc_end_serialized_parallel,
-  /// Call to int32_t __kmpc_shuffle_int32(int32_t element,
-  /// int16_t lane_offset, int16_t warp_size);
-  OMPRTL_NVPTX__kmpc_shuffle_int32,
-  /// Call to int64_t __kmpc_shuffle_int64(int64_t element,
-  /// int16_t lane_offset, int16_t warp_size);
-  OMPRTL_NVPTX__kmpc_shuffle_int64,
-  /// Call to __kmpc_nvptx_parallel_reduce_nowait_v2(ident_t *loc, kmp_int32
-  /// global_tid, kmp_int32 num_vars, size_t reduce_size, void* reduce_data,
-  /// void (*kmp_ShuffleReductFctPtr)(void *rhsData, int16_t lane_id, int16_t
-  /// lane_offset, int16_t shortCircuit),
-  /// void (*kmp_InterWarpCopyFctPtr)(void* src, int32_t warp_num));
-  OMPRTL_NVPTX__kmpc_nvptx_parallel_reduce_nowait_v2,
-  /// Call to __kmpc_nvptx_teams_reduce_nowait_v2(ident_t *loc, kmp_int32
-  /// global_tid, void *global_buffer, int32_t num_of_records, void*
-  /// reduce_data,
-  /// void (*kmp_ShuffleReductFctPtr)(void *rhsData, int16_t lane_id, int16_t
-  /// lane_offset, int16_t shortCircuit),
-  /// void (*kmp_InterWarpCopyFctPtr)(void* src, int32_t warp_num), void
-  /// (*kmp_ListToGlobalCpyFctPtr)(void *buffer, int idx, void *reduce_data),
-  /// void (*kmp_GlobalToListCpyFctPtr)(void *buffer, int idx,
-  /// void *reduce_data), void 

[clang] 9d2378b - [OpenMP] Add Error Handling for Conflicting Pointer Sizes for Target Offload

[via cfe-commits]

Author: Joseph Huber
Date: 2020-09-30T13:58:24-04:00
New Revision: 9d2378b59150f6f1cb5c9cf42ea06b0bb57029a1

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

LOG: [OpenMP] Add Error Handling for Conflicting Pointer Sizes for Target 
Offload

Summary:
This patch adds an error to Clang that detects if OpenMP offloading is used
between two architectures with incompatible pointer sizes. This ensures that
the data mapping can be done correctly and solves an issue in code generation
generating the wrong size pointer.

Reviewer: jdoerfert

Subscribers:

Tags: #OpenMP #Clang

Differential Revision:

Added: 
clang/test/OpenMP/target_incompatible_architecture_messages.cpp

Modified: 
clang/include/clang/Basic/DiagnosticDriverKinds.td
clang/lib/Frontend/CompilerInvocation.cpp
clang/test/OpenMP/nvptx_target_parallel_reduction_codegen_tbaa_PR46146.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticDriverKinds.td 
b/clang/include/clang/Basic/DiagnosticDriverKinds.td
index 3bf1bb19b7ae..29bc19e5a84e 100644
--- a/clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ b/clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -253,6 +253,7 @@ def err_drv_optimization_remark_format : Error<
   "unknown remark serializer format: '%0'">;
 def err_drv_no_neon_modifier : Error<"[no]neon is not accepted as modifier, 
please use [no]simd instead">;
 def err_drv_invalid_omp_target : Error<"OpenMP target is invalid: '%0'">;
+def err_drv_incompatible_omp_arch : Error<"OpenMP target architecture '%0' 
pointer size is incompatible with host '%1'">;
 def err_drv_omp_host_ir_file_not_found : Error<
   "The provided host compiler IR file '%0' is required to generate code for 
OpenMP target regions but cannot be found.">;
 def err_drv_omp_host_target_not_supported : Error<

diff  --git a/clang/lib/Frontend/CompilerInvocation.cpp 
b/clang/lib/Frontend/CompilerInvocation.cpp
index b402f53cc765..bbdf0e3be7ae 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -3206,6 +3206,14 @@ static void ParseLangArgs(LangOptions , ArgList 
, InputKind IK,
 TT.getArch() == llvm::Triple::x86 ||
 TT.getArch() == llvm::Triple::x86_64))
 Diags.Report(diag::err_drv_invalid_omp_target) << A->getValue(i);
+  else if ((T.isArch64Bit() && TT.isArch32Bit()) ||
+   (T.isArch64Bit() && TT.isArch16Bit()) ||
+   (T.isArch32Bit() && TT.isArch64Bit()) ||
+   (T.isArch32Bit() && TT.isArch16Bit()) ||
+   (T.isArch16Bit() && TT.isArch32Bit()) ||
+   (T.isArch16Bit() && TT.isArch64Bit()))
+Diags.Report(diag::err_drv_incompatible_omp_arch)
+<< A->getValue(i) << T.str();
   else
 Opts.OMPTargetTriples.push_back(TT);
 }

diff  --git 
a/clang/test/OpenMP/nvptx_target_parallel_reduction_codegen_tbaa_PR46146.cpp 
b/clang/test/OpenMP/nvptx_target_parallel_reduction_codegen_tbaa_PR46146.cpp
index aefe00f1cadf..031c7b6c778e 100644
--- a/clang/test/OpenMP/nvptx_target_parallel_reduction_codegen_tbaa_PR46146.cpp
+++ b/clang/test/OpenMP/nvptx_target_parallel_reduction_codegen_tbaa_PR46146.cpp
@@ -1,8 +1,8 @@
 // RUN: %clang_cc1 -x c++ -O1 -disable-llvm-optzns -verify -fopenmp 
-internal-isystem %S/../Headers/Inputs/include -internal-isystem 
%S/../../lib/Headers/openmp_wrappers -include __clang_openmp_device_functions.h 
-triple powerpc64le-unknown-unknown -fopenmp-targets=nvptx64-nvidia-cuda 
-emit-llvm-bc %s -o %t-ppc-host.bc
 // RUN: %clang_cc1 -x c++ -O1 -disable-llvm-optzns -verify -fopenmp 
-internal-isystem %S/../Headers/Inputs/include -internal-isystem 
%S/../../lib/Headers/openmp_wrappers -include __clang_openmp_device_functions.h 
-triple nvptx64-unknown-unknown -aux-triple powerpc64le-unknown-unknown  
-fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm %s -fopenmp-is-device 
-fopenmp-host-ir-file-path %t-ppc-host.bc -o - | FileCheck %s
 // RUN: %clang_cc1 -x c++ -O1 -disable-llvm-optzns -verify -fopenmp 
-internal-isystem %S/../Headers/Inputs/include -internal-isystem 
%S/../../lib/Headers/openmp_wrappers -include __clang_openmp_device_functions.h 
-triple i386-unknown-unknown -fopenmp-targets=nvptx-nvidia-cuda -emit-llvm-bc 
%s -o %t-x86-host.bc
-// RUN: %clang_cc1 -x c++ -O1 -disable-llvm-optzns -verify -fopenmp 
-internal-isystem %S/../Headers/Inputs/include -internal-isystem 
%S/../../lib/Headers/openmp_wrappers -include __clang_openmp_device_functions.h 
-triple nvptx-unknown-unknown -aux-triple powerpc64le-unknown-unknown 
-fopenmp-targets=nvptx-nvidia-cuda -emit-llvm %s -fopenmp-is-device 
-fopenmp-host-ir-file-path %t-x86-host.bc -o - | FileCheck %s
-// RUN: %clang_cc1 -x c++ -O1 -disable-llvm-optzns -verify -fopenmp 
-internal-isystem 

[PATCH] D88430: [OpenMP] Replace OpenMP RTL Functions With OMPIRBuilder and OMPKinds.def

[Johannes Doerfert via Phabricator via cfe-commits]

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

LGTM, that should mean OpenMPKinds lists all OpenMP runtime functions clang 
generates, which is great.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88430

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

[clang] 892df30 - Fix interaction of `constinit` and `weak`.

[Richard Smith via cfe-commits]

Author: Richard Smith
Date: 2020-09-30T10:49:50-07:00
New Revision: 892df30a7f344b6cb9995710efbc94bb25cfb95b

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

LOG: Fix interaction of `constinit` and `weak`.

We previously took a shortcut and said that weak variables never have
constant initializers (because those initializers are never correct to
use outside the variable). We now say that weak variables can have
constant initializers, but are never usable in constant expressions.

Added: 
clang/test/SemaCXX/cxx20-constinit.cpp

Modified: 
clang/lib/AST/Decl.cpp
clang/lib/AST/ExprConstant.cpp
clang/lib/Sema/SemaDeclCXX.cpp

Removed: 




diff  --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp
index c96450b8a377..a6c7f30528eb 100644
--- a/clang/lib/AST/Decl.cpp
+++ b/clang/lib/AST/Decl.cpp
@@ -2287,6 +2287,10 @@ bool 
VarDecl::mightBeUsableInConstantExpressions(ASTContext ) const {
   if (isa(this))
 return false;
 
+  // The values of weak variables are never usable in constant expressions.
+  if (isWeak())
+return false;
+
   // In C++11, any variable of reference type can be used in a constant
   // expression if it is initialized by a constant expression.
   if (Lang.CPlusPlus11 && getType()->isReferenceType())
@@ -2414,10 +2418,6 @@ bool VarDecl::isInitICE() const {
 }
 
 bool VarDecl::checkInitIsICE() const {
-  // Initializers of weak variables are never ICEs.
-  if (isWeak())
-return false;
-
   EvaluatedStmt *Eval = ensureEvaluatedStmt();
   if (Eval->CheckedICE)
 // We have already checked whether this subexpression is an

diff  --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index 3bc649b96990..b17eed2dc823 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -14816,7 +14816,7 @@ static ICEDiag CheckICE(const Expr* E, const ASTContext 
) {
 const VarDecl *VD;
 // Look for a declaration of this variable that has an initializer, and
 // check whether it is an ICE.
-if (Dcl->getAnyInitializer(VD) && VD->checkInitIsICE())
+if (Dcl->getAnyInitializer(VD) && !VD->isWeak() && 
VD->checkInitIsICE())
   return NoDiag();
 else
   return ICEDiag(IK_NotICE, cast(E)->getLocation());

diff  --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 2d2b80573a69..1275fc0c95b5 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -2,7 +2,7 @@ QualType 
Sema::CheckComparisonCategoryType(ComparisonCategoryType Kind,
 // might be foobar, including it failing to be a constant expression.
 // TODO Handle more ways the lookup or result can be invalid.
 if (!VD->isStaticDataMember() || !VD->isConstexpr() || !VD->hasInit() ||
-!VD->checkInitIsICE())
+VD->isWeak() || !VD->checkInitIsICE())
   return UnsupportedSTLError(USS_InvalidMember, MemName, VD);
 
 // Attempt to evaluate the var decl as a constant expression and extract

diff  --git a/clang/test/SemaCXX/cxx20-constinit.cpp 
b/clang/test/SemaCXX/cxx20-constinit.cpp
new file mode 100644
index ..a572b9128907
--- /dev/null
+++ b/clang/test/SemaCXX/cxx20-constinit.cpp
@@ -0,0 +1,4 @@
+// RUN: %clang_cc1 %s -std=c++20 -verify
+// expected-no-diagnostics
+
+constinit int a __attribute__((weak)) = 0;



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

[PATCH] D88536: [clangd] Split DecisionForest Evaluate() into one func per tree.

[Adam Czachorowski via Phabricator via cfe-commits]

adamcz added inline comments.



Comment at: clang-tools-extra/clangd/quality/CompletionModelCodegen.py:168
+nline.join(class_members),
+nline.join(["  float EvaluateTree%d() const;" % tree_num
+for tree_num in range(num_trees)]),

Why are these member functions? Why not keep them in .cc file only, in 
anonymous namespace?

I wonder if that will make compiler inline them and then bring back msan issues 
though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88536

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

[PATCH] D88594: [OpenMP] Add Error Handling for Conflicting Pointer Sizes for Target Offload

[Johannes Doerfert via Phabricator via cfe-commits]

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88594

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

[PATCH] D88265: [Sema] Support Comma operator for fp16 vectors.

[Florian Hahn via Phabricator via cfe-commits]

fhahn accepted this revision.
fhahn added a comment.

LGTM, thanks. I'll commit this in a minute.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88265

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

[PATCH] D88265: [Sema] Support Comma operator for fp16 vectors.

[Florian Hahn via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG700e63293eea: [Sema] Support Comma operator for fp16 
vectors. (authored by arames, committed by fhahn).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88265

Files:
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/fp16vec-sema.c


Index: clang/test/Sema/fp16vec-sema.c
===
--- clang/test/Sema/fp16vec-sema.c
+++ clang/test/Sema/fp16vec-sema.c
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wno-unused-value -verify %s
 
 typedef __fp16 half4 __attribute__ ((vector_size (8)));
 typedef float float4 __attribute__ ((vector_size (16)));
@@ -28,6 +28,8 @@
   sv0 = hv0 >= hv1;
   sv0 = hv0 || hv1; // expected-error{{logical expression with vector types 
'half4' (vector of 4 '__fp16' values) and 'half4' is only supported in C++}}
   sv0 = hv0 && hv1; // expected-error{{logical expression with vector types 
'half4' (vector of 4 '__fp16' values) and 'half4' is only supported in C++}}
+  hv0, 1;
+  1, hv0;
 
   // Implicit conversion between half vectors and float vectors are not 
allowed.
   hv0 = fv0; // expected-error{{assigning to}}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -13940,9 +13940,10 @@
   // float vectors and truncating the result back to half vector. For now, we 
do
   // this only when HalfArgsAndReturn is set (that is, when the target is arm 
or
   // arm64).
-  assert(isVector(RHS.get()->getType(), Context.HalfTy) ==
- isVector(LHS.get()->getType(), Context.HalfTy) &&
- "both sides are half vectors or neither sides are");
+  assert(
+  (Opc == BO_Comma || isVector(RHS.get()->getType(), Context.HalfTy) ==
+  isVector(LHS.get()->getType(), Context.HalfTy)) 
&&
+  "both sides are half vectors or neither sides are");
   ConvertHalfVec =
   needsConversionOfHalfVec(ConvertHalfVec, Context, LHS.get(), RHS.get());
 


Index: clang/test/Sema/fp16vec-sema.c
===
--- clang/test/Sema/fp16vec-sema.c
+++ clang/test/Sema/fp16vec-sema.c
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wno-unused-value -verify %s
 
 typedef __fp16 half4 __attribute__ ((vector_size (8)));
 typedef float float4 __attribute__ ((vector_size (16)));
@@ -28,6 +28,8 @@
   sv0 = hv0 >= hv1;
   sv0 = hv0 || hv1; // expected-error{{logical expression with vector types 'half4' (vector of 4 '__fp16' values) and 'half4' is only supported in C++}}
   sv0 = hv0 && hv1; // expected-error{{logical expression with vector types 'half4' (vector of 4 '__fp16' values) and 'half4' is only supported in C++}}
+  hv0, 1;
+  1, hv0;
 
   // Implicit conversion between half vectors and float vectors are not allowed.
   hv0 = fv0; // expected-error{{assigning to}}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -13940,9 +13940,10 @@
   // float vectors and truncating the result back to half vector. For now, we do
   // this only when HalfArgsAndReturn is set (that is, when the target is arm or
   // arm64).
-  assert(isVector(RHS.get()->getType(), Context.HalfTy) ==
- isVector(LHS.get()->getType(), Context.HalfTy) &&
- "both sides are half vectors or neither sides are");
+  assert(
+  (Opc == BO_Comma || isVector(RHS.get()->getType(), Context.HalfTy) ==
+  isVector(LHS.get()->getType(), Context.HalfTy)) &&
+  "both sides are half vectors or neither sides are");
   ConvertHalfVec =
   needsConversionOfHalfVec(ConvertHalfVec, Context, LHS.get(), RHS.get());
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 700e632 - [Sema] Support Comma operator for fp16 vectors.

[Florian Hahn via cfe-commits]

Author: Alexandre Rames
Date: 2020-09-30T18:23:09+01:00
New Revision: 700e63293eea4a23440f300b1e9125ca2e80c6e9

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

LOG: [Sema] Support Comma operator for fp16 vectors.

The current half vector was enforcing an assert expecting
 "(LHS is half vector) == (RHS is half vector)"
for comma.

Reviewed By: ahatanak, fhahn

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

Added: 


Modified: 
clang/lib/Sema/SemaExpr.cpp
clang/test/Sema/fp16vec-sema.c

Removed: 




diff  --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index a7c076657fb5..22840dd3dfe3 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -13940,9 +13940,10 @@ ExprResult Sema::CreateBuiltinBinOp(SourceLocation 
OpLoc,
   // float vectors and truncating the result back to half vector. For now, we 
do
   // this only when HalfArgsAndReturn is set (that is, when the target is arm 
or
   // arm64).
-  assert(isVector(RHS.get()->getType(), Context.HalfTy) ==
- isVector(LHS.get()->getType(), Context.HalfTy) &&
- "both sides are half vectors or neither sides are");
+  assert(
+  (Opc == BO_Comma || isVector(RHS.get()->getType(), Context.HalfTy) ==
+  isVector(LHS.get()->getType(), Context.HalfTy)) 
&&
+  "both sides are half vectors or neither sides are");
   ConvertHalfVec =
   needsConversionOfHalfVec(ConvertHalfVec, Context, LHS.get(), RHS.get());
 

diff  --git a/clang/test/Sema/fp16vec-sema.c b/clang/test/Sema/fp16vec-sema.c
index aefb5f86a14b..f61ad4c91e89 100644
--- a/clang/test/Sema/fp16vec-sema.c
+++ b/clang/test/Sema/fp16vec-sema.c
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wno-unused-value -verify %s
 
 typedef __fp16 half4 __attribute__ ((vector_size (8)));
 typedef float float4 __attribute__ ((vector_size (16)));
@@ -28,6 +28,8 @@ void testFP16Vec(int c) {
   sv0 = hv0 >= hv1;
   sv0 = hv0 || hv1; // expected-error{{logical expression with vector types 
'half4' (vector of 4 '__fp16' values) and 'half4' is only supported in C++}}
   sv0 = hv0 && hv1; // expected-error{{logical expression with vector types 
'half4' (vector of 4 '__fp16' values) and 'half4' is only supported in C++}}
+  hv0, 1;
+  1, hv0;
 
   // Implicit conversion between half vectors and float vectors are not 
allowed.
   hv0 = fv0; // expected-error{{assigning to}}



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

[PATCH] D88430: [OpenMP] Replace OpenMP RTL Functions With OMPIRBuilder and OMPKinds.def

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 updated this revision to Diff 295343.
jhuber6 added a comment.

Forgot to add one of the test changes from D88594 
 as it makes this one fail too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88430

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.h
  clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/test/OpenMP/nvptx_parallel_codegen.cpp
  clang/test/OpenMP/nvptx_target_parallel_reduction_codegen_tbaa_PR46146.cpp
  llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
  llvm/test/Transforms/OpenMP/add_attributes.ll

Index: llvm/test/Transforms/OpenMP/add_attributes.ll
===
--- llvm/test/Transforms/OpenMP/add_attributes.ll
+++ llvm/test/Transforms/OpenMP/add_attributes.ll
@@ -888,313 +888,313 @@
 ; CHECK: declare dso_local i32 @omp_pause_resource_all(i32)
 
 ; CHECK: ; Function Attrs: nounwind
-; CHECK-NEXT: declare dso_local i32 @omp_get_supported_active_levels() #0
+; CHECK-NEXT: declare dso_local i32 @omp_get_supported_active_levels()
 
-; CHECK: ; Function Attrs: nounwind
-; CHECK-NEXT: declare void @__kmpc_barrier(%struct.ident_t*, i32) #0
+; CHECK: ; Function Attrs: convergent nounwind
+; CHECK-NEXT: declare void @__kmpc_barrier(%struct.ident_t*, i32)
 
 ; CHECK: ; Function Attrs: nounwind
-; CHECK-NEXT: declare i32 @__kmpc_cancel(%struct.ident_t*, i32, i32) #0
+; CHECK-NEXT: declare i32 @__kmpc_cancel(%struct.ident_t*, i32, i32)
 
-; CHECK: ; Function Attrs: nounwind
-; CHECK-NEXT: declare i32 @__kmpc_cancel_barrier(%struct.ident_t*, i32) #0
+; CHECK: ; Function Attrs: convergent nounwind
+; CHECK-NEXT: declare i32 @__kmpc_cancel_barrier(%struct.ident_t*, i32)
 
-; CHECK: ; Function Attrs: nounwind
-; CHECK-NEXT: declare void @__kmpc_flush(%struct.ident_t*) #0
+; CHECK: ; Function Attrs: convergent nounwind
+; CHECK-NEXT: declare void @__kmpc_flush(%struct.ident_t*)
 
 ; CHECK: ; Function Attrs: nounwind
-; CHECK-NEXT: declare i32 @__kmpc_global_thread_num(%struct.ident_t*) #0
+; CHECK-NEXT: declare i32 @__kmpc_global_thread_num(%struct.ident_t*)
 
 ; CHECK: ; Function Attrs: nounwind
-; CHECK-NEXT: declare void @__kmpc_fork_call(%struct.ident_t*, i32, void (i32*, i32*, ...)*, ...) #0
+; CHECK-NEXT: declare void @__kmpc_fork_call(%struct.ident_t*, i32, void (i32*, i32*, ...)*, ...)
 
-; CHECK: ; Function Attrs: nounwind
-; CHECK-NEXT: declare i32 @__kmpc_omp_taskwait(%struct.ident_t*, i32) #0
+; CHECK: ; Function Attrs: convergent nounwind
+; CHECK-NEXT: declare i32 @__kmpc_omp_taskwait(%struct.ident_t*, i32)
 
 ; CHECK: ; Function Attrs: nounwind
-; CHECK-NEXT: declare i32 @__kmpc_omp_taskyield(%struct.ident_t*, i32, i32) #0
+; CHECK-NEXT: declare i32 @__kmpc_omp_taskyield(%struct.ident_t*, i32, i32)
 
 ; CHECK: ; Function Attrs: nounwind
-; CHECK-NEXT: declare void @__kmpc_push_num_threads(%struct.ident_t*, i32, i32) #0
+; CHECK-NEXT: declare void @__kmpc_push_num_threads(%struct.ident_t*, i32, i32)
 
 ; CHECK: ; Function Attrs: nounwind
-; CHECK-NEXT: declare void @__kmpc_push_proc_bind(%struct.ident_t*, i32, i32) #0
+; CHECK-NEXT: declare void @__kmpc_push_proc_bind(%struct.ident_t*, i32, i32)
 
 ; CHECK: ; Function Attrs: nounwind
-; CHECK-NEXT: declare void @__kmpc_serialized_parallel(%struct.ident_t*, i32) #0
+; CHECK-NEXT: declare void @__kmpc_serialized_parallel(%struct.ident_t*, i32)
 
 ; CHECK: ; Function Attrs: nounwind
-; CHECK-NEXT: declare void @__kmpc_end_serialized_parallel(%struct.ident_t*, i32) #0
+; CHECK-NEXT: declare void @__kmpc_end_serialized_parallel(%struct.ident_t*, i32)
 
 ; CHECK: ; Function Attrs: nounwind
-; CHECK-NEXT: declare i32 @__kmpc_master(%struct.ident_t*, i32) #0
+; CHECK-NEXT: declare i32 @__kmpc_master(%struct.ident_t*, i32)
 
 ; CHECK: ; Function Attrs: nounwind
-; CHECK-NEXT: declare void @__kmpc_end_master(%struct.ident_t*, i32) #0
+; CHECK-NEXT: declare void @__kmpc_end_master(%struct.ident_t*, i32)
 
-; CHECK: ; Function Attrs: nounwind
-; CHECK-NEXT: declare void @__kmpc_critical(%struct.ident_t*, i32, [8 x i32]*) #0
+; CHECK: ; Function Attrs: convergent nounwind
+; CHECK-NEXT: declare void @__kmpc_critical(%struct.ident_t*, i32, [8 x i32]*)
 
-; CHECK: ; Function Attrs: nounwind
-; CHECK-NEXT: declare void @__kmpc_critical_with_hint(%struct.ident_t*, i32, [8 x i32]*, i32) #0
+; CHECK: ; Function Attrs: convergent nounwind
+; CHECK-NEXT: declare void @__kmpc_critical_with_hint(%struct.ident_t*, i32, [8 x i32]*, i32)
 
-; CHECK: ; Function Attrs: nounwind
-; CHECK-NEXT: declare void @__kmpc_end_critical(%struct.ident_t*, i32, [8 x i32]*) #0
+; CHECK: ; Function Attrs: convergent nounwind
+; CHECK-NEXT: declare void @__kmpc_end_critical(%struct.ident_t*, i32, [8 x i32]*)
 
 ; CHECK: ; Function Attrs: nounwind
-; CHECK-NEXT: declare void @__kmpc_begin(%struct.ident_t*, i32) #0
+; CHECK-NEXT: declare void @__kmpc_begin(%struct.ident_t*, i32)
 
 

[clang] 187686b - [CodeGen] add test for NAN creation; NFC

[Sanjay Patel via cfe-commits]

Author: Sanjay Patel
Date: 2020-09-30T13:22:12-04:00
New Revision: 187686bea3878c0bf2b150d784e7eab223434e25

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

LOG: [CodeGen] add test for NAN creation; NFC

This goes with the APFloat change proposed in
D88238.
This is copied from the MIPS-specific test in
builtin-nan-legacy.c to verify that the normal
behavior is correct on other targets without the
complication of an inverted quiet bit.

Added: 
clang/test/CodeGen/builtin-nan-exception.c

Modified: 


Removed: 




diff  --git a/clang/test/CodeGen/builtin-nan-exception.c 
b/clang/test/CodeGen/builtin-nan-exception.c
new file mode 100644
index ..2acf0c4390ec
--- /dev/null
+++ b/clang/test/CodeGen/builtin-nan-exception.c
@@ -0,0 +1,23 @@
+// RUN: %clang -target aarch64 -emit-llvm -S %s -o - | FileCheck %s
+// RUN: %clang -target lanai -emit-llvm -S %s -o - | FileCheck %s
+// RUN: %clang -target riscv64 -emit-llvm -S %s -o - | FileCheck %s
+// RUN: %clang -target x86_64 -emit-llvm -S %s -o - | FileCheck %s
+
+// Run a variety of targets to ensure there's no target-based 
diff erence.
+
+// The builtin always produces a 64-bit (double).
+// An SNaN with no payload is formed by setting the bit after the
+// the quiet bit (MSB of the significand).
+
+// CHECK: float 0x7FF8, float 0x7FF4
+// CHECK: double 0x7FF8, double 0x7FF4
+
+float f[] = {
+  __builtin_nan(""),
+  __builtin_nans(""),
+};
+
+double d[] = {
+  __builtin_nan(""),
+  __builtin_nans(""),
+};



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

[PATCH] D88430: [OpenMP] Replace OpenMP RTL Functions With OMPIRBuilder and OMPKinds.def

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 updated this revision to Diff 295342.
jhuber6 added a comment.

Moving the Clang error checks into D88594 .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88430

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.h
  clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/test/OpenMP/nvptx_parallel_codegen.cpp
  llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
  llvm/test/Transforms/OpenMP/add_attributes.ll

Index: llvm/test/Transforms/OpenMP/add_attributes.ll
===
--- llvm/test/Transforms/OpenMP/add_attributes.ll
+++ llvm/test/Transforms/OpenMP/add_attributes.ll
@@ -888,313 +888,313 @@
 ; CHECK: declare dso_local i32 @omp_pause_resource_all(i32)
 
 ; CHECK: ; Function Attrs: nounwind
-; CHECK-NEXT: declare dso_local i32 @omp_get_supported_active_levels() #0
+; CHECK-NEXT: declare dso_local i32 @omp_get_supported_active_levels()
 
-; CHECK: ; Function Attrs: nounwind
-; CHECK-NEXT: declare void @__kmpc_barrier(%struct.ident_t*, i32) #0
+; CHECK: ; Function Attrs: convergent nounwind
+; CHECK-NEXT: declare void @__kmpc_barrier(%struct.ident_t*, i32)
 
 ; CHECK: ; Function Attrs: nounwind
-; CHECK-NEXT: declare i32 @__kmpc_cancel(%struct.ident_t*, i32, i32) #0
+; CHECK-NEXT: declare i32 @__kmpc_cancel(%struct.ident_t*, i32, i32)
 
-; CHECK: ; Function Attrs: nounwind
-; CHECK-NEXT: declare i32 @__kmpc_cancel_barrier(%struct.ident_t*, i32) #0
+; CHECK: ; Function Attrs: convergent nounwind
+; CHECK-NEXT: declare i32 @__kmpc_cancel_barrier(%struct.ident_t*, i32)
 
-; CHECK: ; Function Attrs: nounwind
-; CHECK-NEXT: declare void @__kmpc_flush(%struct.ident_t*) #0
+; CHECK: ; Function Attrs: convergent nounwind
+; CHECK-NEXT: declare void @__kmpc_flush(%struct.ident_t*)
 
 ; CHECK: ; Function Attrs: nounwind
-; CHECK-NEXT: declare i32 @__kmpc_global_thread_num(%struct.ident_t*) #0
+; CHECK-NEXT: declare i32 @__kmpc_global_thread_num(%struct.ident_t*)
 
 ; CHECK: ; Function Attrs: nounwind
-; CHECK-NEXT: declare void @__kmpc_fork_call(%struct.ident_t*, i32, void (i32*, i32*, ...)*, ...) #0
+; CHECK-NEXT: declare void @__kmpc_fork_call(%struct.ident_t*, i32, void (i32*, i32*, ...)*, ...)
 
-; CHECK: ; Function Attrs: nounwind
-; CHECK-NEXT: declare i32 @__kmpc_omp_taskwait(%struct.ident_t*, i32) #0
+; CHECK: ; Function Attrs: convergent nounwind
+; CHECK-NEXT: declare i32 @__kmpc_omp_taskwait(%struct.ident_t*, i32)
 
 ; CHECK: ; Function Attrs: nounwind
-; CHECK-NEXT: declare i32 @__kmpc_omp_taskyield(%struct.ident_t*, i32, i32) #0
+; CHECK-NEXT: declare i32 @__kmpc_omp_taskyield(%struct.ident_t*, i32, i32)
 
 ; CHECK: ; Function Attrs: nounwind
-; CHECK-NEXT: declare void @__kmpc_push_num_threads(%struct.ident_t*, i32, i32) #0
+; CHECK-NEXT: declare void @__kmpc_push_num_threads(%struct.ident_t*, i32, i32)
 
 ; CHECK: ; Function Attrs: nounwind
-; CHECK-NEXT: declare void @__kmpc_push_proc_bind(%struct.ident_t*, i32, i32) #0
+; CHECK-NEXT: declare void @__kmpc_push_proc_bind(%struct.ident_t*, i32, i32)
 
 ; CHECK: ; Function Attrs: nounwind
-; CHECK-NEXT: declare void @__kmpc_serialized_parallel(%struct.ident_t*, i32) #0
+; CHECK-NEXT: declare void @__kmpc_serialized_parallel(%struct.ident_t*, i32)
 
 ; CHECK: ; Function Attrs: nounwind
-; CHECK-NEXT: declare void @__kmpc_end_serialized_parallel(%struct.ident_t*, i32) #0
+; CHECK-NEXT: declare void @__kmpc_end_serialized_parallel(%struct.ident_t*, i32)
 
 ; CHECK: ; Function Attrs: nounwind
-; CHECK-NEXT: declare i32 @__kmpc_master(%struct.ident_t*, i32) #0
+; CHECK-NEXT: declare i32 @__kmpc_master(%struct.ident_t*, i32)
 
 ; CHECK: ; Function Attrs: nounwind
-; CHECK-NEXT: declare void @__kmpc_end_master(%struct.ident_t*, i32) #0
+; CHECK-NEXT: declare void @__kmpc_end_master(%struct.ident_t*, i32)
 
-; CHECK: ; Function Attrs: nounwind
-; CHECK-NEXT: declare void @__kmpc_critical(%struct.ident_t*, i32, [8 x i32]*) #0
+; CHECK: ; Function Attrs: convergent nounwind
+; CHECK-NEXT: declare void @__kmpc_critical(%struct.ident_t*, i32, [8 x i32]*)
 
-; CHECK: ; Function Attrs: nounwind
-; CHECK-NEXT: declare void @__kmpc_critical_with_hint(%struct.ident_t*, i32, [8 x i32]*, i32) #0
+; CHECK: ; Function Attrs: convergent nounwind
+; CHECK-NEXT: declare void @__kmpc_critical_with_hint(%struct.ident_t*, i32, [8 x i32]*, i32)
 
-; CHECK: ; Function Attrs: nounwind
-; CHECK-NEXT: declare void @__kmpc_end_critical(%struct.ident_t*, i32, [8 x i32]*) #0
+; CHECK: ; Function Attrs: convergent nounwind
+; CHECK-NEXT: declare void @__kmpc_end_critical(%struct.ident_t*, i32, [8 x i32]*)
 
 ; CHECK: ; Function Attrs: nounwind
-; CHECK-NEXT: declare void @__kmpc_begin(%struct.ident_t*, i32) #0
+; CHECK-NEXT: declare void @__kmpc_begin(%struct.ident_t*, i32)
 
 ; CHECK: ; Function Attrs: nounwind
-; CHECK-NEXT: declare void @__kmpc_end(%struct.ident_t*) #0
+; CHECK-NEXT: 

[PATCH] D88594: [OpenMP] Add Error Handling for Conflicting Pointer Sizes for Target Offload

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 created this revision.
jhuber6 added a reviewer: jdoerfert.
jhuber6 added projects: OpenMP, clang.
Herald added subscribers: cfe-commits, guansong, yaxunl.
jhuber6 requested review of this revision.
Herald added a subscriber: sstefan1.

This patch adds an error to Clang that detects if OpenMP offloading is used 
between two architectures with incompatible pointer sizes. This ensures that 
the data mapping can be done correctly and solves an issue in code generation 
generating the wrong size pointer.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D88594

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/OpenMP/nvptx_target_parallel_reduction_codegen_tbaa_PR46146.cpp
  clang/test/OpenMP/target_incompatible_architecture_messages.cpp


Index: clang/test/OpenMP/target_incompatible_architecture_messages.cpp
===
--- /dev/null
+++ clang/test/OpenMP/target_incompatible_architecture_messages.cpp
@@ -0,0 +1,14 @@
+// RUN: not %clang_cc1 -x c++ -fopenmp -triple powerpc64le-unknown-unknown 
-fopenmp-targets=nvptx-nvidia-cuda -o - %s 2>&1 | FileCheck %s
+// RUN: not %clang_cc1 -x c++ -fopenmp -triple i386-unknown-unknown 
-fopenmp-targets=nvptx64-nvidia-cuda -o - %s 2>&1 | FileCheck %s
+// RUN: not %clang_cc1 -x c++ -fopenmp -triple x86_64-unknown-unknown 
-fopenmp-targets=nvptx-nvidia-cuda -o - %s 2>&1 | FileCheck %s
+// RUN: not %clang_cc1 -x c++ -fopenmp -triple x86_64-unknown-unknown 
-fopenmp-targets=i386-pc-linux-gnu -o - %s 2>&1 | FileCheck %s
+// CHECK: error: OpenMP target architecture '{{.+}}' pointer size is 
incompatible with host '{{.+}}'
+#ifndef HEADER
+#define HEADER
+
+void test() {
+#pragma omp target
+  {}
+}
+
+#endif
Index: 
clang/test/OpenMP/nvptx_target_parallel_reduction_codegen_tbaa_PR46146.cpp
===
--- clang/test/OpenMP/nvptx_target_parallel_reduction_codegen_tbaa_PR46146.cpp
+++ clang/test/OpenMP/nvptx_target_parallel_reduction_codegen_tbaa_PR46146.cpp
@@ -1,8 +1,8 @@
 // RUN: %clang_cc1 -x c++ -O1 -disable-llvm-optzns -verify -fopenmp 
-internal-isystem %S/../Headers/Inputs/include -internal-isystem 
%S/../../lib/Headers/openmp_wrappers -include __clang_openmp_device_functions.h 
-triple powerpc64le-unknown-unknown -fopenmp-targets=nvptx64-nvidia-cuda 
-emit-llvm-bc %s -o %t-ppc-host.bc
 // RUN: %clang_cc1 -x c++ -O1 -disable-llvm-optzns -verify -fopenmp 
-internal-isystem %S/../Headers/Inputs/include -internal-isystem 
%S/../../lib/Headers/openmp_wrappers -include __clang_openmp_device_functions.h 
-triple nvptx64-unknown-unknown -aux-triple powerpc64le-unknown-unknown  
-fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm %s -fopenmp-is-device 
-fopenmp-host-ir-file-path %t-ppc-host.bc -o - | FileCheck %s
 // RUN: %clang_cc1 -x c++ -O1 -disable-llvm-optzns -verify -fopenmp 
-internal-isystem %S/../Headers/Inputs/include -internal-isystem 
%S/../../lib/Headers/openmp_wrappers -include __clang_openmp_device_functions.h 
-triple i386-unknown-unknown -fopenmp-targets=nvptx-nvidia-cuda -emit-llvm-bc 
%s -o %t-x86-host.bc
-// RUN: %clang_cc1 -x c++ -O1 -disable-llvm-optzns -verify -fopenmp 
-internal-isystem %S/../Headers/Inputs/include -internal-isystem 
%S/../../lib/Headers/openmp_wrappers -include __clang_openmp_device_functions.h 
-triple nvptx-unknown-unknown -aux-triple powerpc64le-unknown-unknown 
-fopenmp-targets=nvptx-nvidia-cuda -emit-llvm %s -fopenmp-is-device 
-fopenmp-host-ir-file-path %t-x86-host.bc -o - | FileCheck %s
-// RUN: %clang_cc1 -x c++ -O1 -disable-llvm-optzns -verify -fopenmp 
-internal-isystem %S/../Headers/Inputs/include -internal-isystem 
%S/../../lib/Headers/openmp_wrappers -include __clang_openmp_device_functions.h 
-fexceptions -fcxx-exceptions -aux-triple powerpc64le-unknown-unknown -triple 
nvptx-unknown-unknown -fopenmp-targets=nvptx-nvidia-cuda -emit-llvm %s 
-fopenmp-is-device -fopenmp-host-ir-file-path %t-x86-host.bc -o - | FileCheck %s
+// RUN: %clang_cc1 -x c++ -O1 -disable-llvm-optzns -verify -fopenmp 
-internal-isystem %S/../Headers/Inputs/include -internal-isystem 
%S/../../lib/Headers/openmp_wrappers -include __clang_openmp_device_functions.h 
-triple nvptx64-unknown-unknown -aux-triple powerpc64le-unknown-unknown 
-fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm %s -fopenmp-is-device 
-fopenmp-host-ir-file-path %t-x86-host.bc -o - | FileCheck %s
+// RUN: %clang_cc1 -x c++ -O1 -disable-llvm-optzns -verify -fopenmp 
-internal-isystem %S/../Headers/Inputs/include -internal-isystem 
%S/../../lib/Headers/openmp_wrappers -include __clang_openmp_device_functions.h 
-fexceptions -fcxx-exceptions -aux-triple powerpc64le-unknown-unknown -triple 
nvptx64-unknown-unknown -fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm %s 
-fopenmp-is-device -fopenmp-host-ir-file-path %t-x86-host.bc -o - | FileCheck %s
 // expected-no-diagnostics
 #ifndef HEADER
 

[PATCH] D88590: [clangd] Add bencmark for measuring latency of DecisionForest model.

[Utkarsh Saxena via Phabricator via cfe-commits]

usaxena95 updated this revision to Diff 295338.
usaxena95 added a comment.

Minor fixes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88590

Files:
  clang-tools-extra/clangd/benchmarks/CMakeLists.txt
  clang-tools-extra/clangd/benchmarks/DecisionForestBenchmark.cpp


Index: clang-tools-extra/clangd/benchmarks/DecisionForestBenchmark.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/benchmarks/DecisionForestBenchmark.cpp
@@ -0,0 +1,57 @@
+//===--- DecisionForestBenchmark.cpp - Benchmark for code completion ranking
+//latency ---*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "CompletionModel.h"
+#include "benchmark/benchmark.h"
+
+const char *IndexFilename;
+const char *RequestsFilename;
+
+namespace clang {
+namespace clangd {
+namespace {
+void RunDecisionForestPrediciton() {
+  Example E;
+
+  // E.setIsDeprecated(Quality.Deprecated);
+  // E.setIsReservedName(Quality.ReservedName);
+  // E.setIsImplementationDetail(Quality.ImplementationDetail);
+  // E.setNumReferences(Quality.References);
+  // E.setSymbolCategory(Quality.Category);
+
+  // E.setIsNameInContext(Derived.NameMatchesContext);
+  // E.setIsForbidden(Relevance.Forbidden);
+  // E.setIsInBaseClass(Relevance.InBaseClass);
+  // E.setFileProximityDistance(Derived.FileProximityDistance);
+  // E.setSemaFileProximityScore(Relevance.SemaFileProximityScore);
+  // E.setSymbolScopeDistance(Derived.ScopeProximityDistance);
+  // E.setSemaSaysInScope(Relevance.SemaSaysInScope);
+  // E.setScope(Relevance.Scope);
+  // E.setContextKind(Relevance.Context);
+  // E.setIsInstanceMember(Relevance.IsInstanceMember);
+  // E.setHadContextType(Relevance.HadContextType);
+  // E.setHadSymbolType(Relevance.HadSymbolType);
+  // E.setTypeMatchesPreferred(Relevance.TypeMatchesPreferred);
+  // E.setFilterLength(Relevance.FilterLength);
+  return Evaluate(E);
+}
+static void DecisionForestPredict(benchmark::State ) {
+  for (auto _ : State)
+RunDecisionForestPrediciton();
+}
+BENCHMARK(DecisionForestPredict);
+
+} // namespace
+} // namespace clangd
+} // namespace clang
+
+int main(int argc, char *argv[]) {
+  ::benchmark::Initialize(, argv);
+  ::benchmark::RunSpecifiedBenchmarks();
+}
Index: clang-tools-extra/clangd/benchmarks/CMakeLists.txt
===
--- clang-tools-extra/clangd/benchmarks/CMakeLists.txt
+++ clang-tools-extra/clangd/benchmarks/CMakeLists.txt
@@ -7,3 +7,11 @@
   clangDaemon
   LLVMSupport
   )
+
+add_benchmark(DecisionForestBenchmark DecisionForestBenchmark.cpp)
+
+target_link_libraries(DecisionForestBenchmark
+  PRIVATE
+  clangDaemon
+  LLVMSupport
+  )
\ No newline at end of file


Index: clang-tools-extra/clangd/benchmarks/DecisionForestBenchmark.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/benchmarks/DecisionForestBenchmark.cpp
@@ -0,0 +1,57 @@
+//===--- DecisionForestBenchmark.cpp - Benchmark for code completion ranking
+//latency ---*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "CompletionModel.h"
+#include "benchmark/benchmark.h"
+
+const char *IndexFilename;
+const char *RequestsFilename;
+
+namespace clang {
+namespace clangd {
+namespace {
+void RunDecisionForestPrediciton() {
+  Example E;
+
+  // E.setIsDeprecated(Quality.Deprecated);
+  // E.setIsReservedName(Quality.ReservedName);
+  // E.setIsImplementationDetail(Quality.ImplementationDetail);
+  // E.setNumReferences(Quality.References);
+  // E.setSymbolCategory(Quality.Category);
+
+  // E.setIsNameInContext(Derived.NameMatchesContext);
+  // E.setIsForbidden(Relevance.Forbidden);
+  // E.setIsInBaseClass(Relevance.InBaseClass);
+  // E.setFileProximityDistance(Derived.FileProximityDistance);
+  // E.setSemaFileProximityScore(Relevance.SemaFileProximityScore);
+  // E.setSymbolScopeDistance(Derived.ScopeProximityDistance);
+  // E.setSemaSaysInScope(Relevance.SemaSaysInScope);
+  // E.setScope(Relevance.Scope);
+  // E.setContextKind(Relevance.Context);
+  // E.setIsInstanceMember(Relevance.IsInstanceMember);
+  // E.setHadContextType(Relevance.HadContextType);
+  // E.setHadSymbolType(Relevance.HadSymbolType);
+  // E.setTypeMatchesPreferred(Relevance.TypeMatchesPreferred);
+  // E.setFilterLength(Relevance.FilterLength);
+  return 

[PATCH] D88265: [Sema] Support Comma operator for fp16 vectors.

[Alexandre Rames via Phabricator via cfe-commits]

arames added a comment.



> Oh I think you'd need to edit the revision on Phabricator (top right on this 
> page, `edit revision`). But if you are fine with it I can commit the patch 
> for you with the adjusted commit title.

Done !
Thanks for your help !


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88265

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

[PATCH] D88265: Fix comma with half vectors.

[Florian Hahn via Phabricator via cfe-commits]

fhahn added a comment.

In D88265#2303841 , @arames wrote:

> In D88265#2302653 , @fhahn wrote:
>
>> Could you adjust the commit message to be a bit more descriptive, e.g 
>> something like `[Sema] Support Comma operator for fp16 vectors.`
>
> Done!

Oh I think you'd need to edit the revision on Phabricator (top right on this 
page, `edit revision`). But if you are fine with it I can commit the patch for 
you with the adjusted commit title.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88265

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

[PATCH] D86671: [clang-tidy] Add new case type to check variables with Hungarian notation

[Douglas Chen via Phabricator via cfe-commits]

dougpuob added a comment.

In D86671#2293128 , @aaron.ballman 
wrote:

> In D86671#2284078 , @dougpuob wrote:
>
>> Hi @aaron.ballman
>> About changing  `size_t nLength` to `cbLength`. I searched MSVC folders with 
>> `size_t`, many names of variable start with `n`, or `i` in MFC related 
>> files. So I prefer to keep it starts with `n`. Another side to name starts 
>> with `cb`, I found variables like cbXxx are defined with ULONG, DWORD, or 
>> UCHAR type.
>
> I think the important point is that `cb` is used for APIs that specify a 
> count of bytes, whereas `i`, `n`, and `l` are used for integers (there is no 
> specific prefix for `size_t` that I'm aware of or that MSDN documents, so the 
> lack of consistency there is not too surprising). That's why I mentioned that 
> using a heuristic approach may allow you to identify the parameters that are 
> intended to be a count of bytes so that you can use the more specific prefix 
> if there's more specific contextual information available.

Hi @aaron.ballman

1. **Making `sz` as a prefix to the `char*` parameters of `strncpy()` and 
`strncat()` functions.** I read the code of `strncpy` function, seems 
null-terminated to parameters is essential. I thought those parameters to 
strXxx() are also able to the parameter of `strlen` function knowing the length 
of a C string(it can be passed to be a parameter or assigned to be a variable, 
those point to the identical memory. strXxx functions tell the end by null 
character in the memory). Moreover, I searched source code of OpenSSL, I found 
the project uses most of `char*` as C string, and data buffer to retrieve data 
with `unsigned char*`.  `unsigned char` is a primitive type in C89 (is portable 
than `uint8_t`). Maybe current mechanism is a good way to hint users that 
`unsigned char*` is more explicit than `char*` if they want to treat it as a 
buffer to retrieve data.

2. **Adding heuristics to pick the correct prefix.** Do you mean that use the 
heuristics approach to give naming suggestion as warning message, or correct it 
with the `--fix` option? Is there any existing in this project can be a sample 
for me? If my thought about the heuristics is correct. The information of 
parameters to functions I can query its **type**, **name**, and **location**. 
As we have discussed that the declaration type is insufficient to tell `sz` for 
`char*`, or `cb` for count of bytes instead `i`, `n`, or `l`, so the **name** 
and **location** provide more information for comparing names with string 
mapping tables and location relationship between parameters/variables. Take 
`FILE * fopen ( const char * filename, const char * mode );` for example(C 
String). It will change `filename` to `szFilename` if the `name` string is in 
the mapping table, change `mode` to `pcMode` if `mode` string is not in the 
mapping table. Take `size_t fread ( void * ptr, size_t size, size_t count, FILE 
* stream );` for example(count of bytes). It will change `size` to `cbSize`, 
because its previous parameter is a void pointer.

  I can smell that there is always exceptional cases, and not good for 
maintainability.

3. **Changing `i`, `n`, and `l` to `cb` for APIs that specify a count of 
bytes.** I have no idea how to distinguish when should add the `cb` instead of 
integer types for heuristics. If I was the user, I wish clang-tidy don't change 
integer types from `cbXxx` to `iCbXxx` or `nCbXxx`. Keep `cb` with default or 
an option in config file, like `IgnoreParameterPrefix`, `IgnoreVariablePrefix`.

4. **Why not use Decl->getType()->getAsString()** I printed the differences as 
a log for your reference. 
(https://gist.github.com/dougpuob/e5a76db6e2c581ba003afec3236ab6ac#file-clang-tidy-readability-identifier-naming-hungarian-notation-log-L191)
 Previous I mentioned "if users haven't specific the include directories, the 
checking result may look messy". This concern will not happened because the 
`clang-diagnostic-error`, if users didn't specific header directories, 
clang-tidy will show the error (L414 
).




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86671

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

[PATCH] D88590: [clangd] Add bencmark for measuring latency of DecisionForest model.

[Utkarsh Saxena via Phabricator via cfe-commits]

usaxena95 created this revision.
Herald added subscribers: cfe-commits, kadircet, arphaman, mgorny.
Herald added a project: clang.
usaxena95 requested review of this revision.
Herald added subscribers: MaskRay, ilya-biryukov.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D88590

Files:
  clang-tools-extra/clangd/benchmarks/CMakeLists.txt
  clang-tools-extra/clangd/benchmarks/DecisionForestBenchmark.cpp


Index: clang-tools-extra/clangd/benchmarks/DecisionForestBenchmark.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/benchmarks/DecisionForestBenchmark.cpp
@@ -0,0 +1,59 @@
+//===--- IndexBenchmark.cpp - Clangd index benchmarks ---*- C++ 
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "CompletionModel.h"
+#include "benchmark/benchmark.h"
+
+const char *IndexFilename;
+const char *RequestsFilename;
+
+namespace clang {
+namespace clangd {
+namespace {
+void RunDecisionForestPredicitons() {
+  constexpr int NumExperiments = 1;
+  for (int I = 0; I < NumExperiments; ++I) {
+Example E;
+
+E.setIsDeprecated(Quality.Deprecated);
+E.setIsReservedName(Quality.ReservedName);
+E.setIsImplementationDetail(Quality.ImplementationDetail);
+E.setNumReferences(Quality.References);
+E.setSymbolCategory(Quality.Category);
+
+E.setIsNameInContext(Derived.NameMatchesContext);
+E.setIsForbidden(Relevance.Forbidden);
+E.setIsInBaseClass(Relevance.InBaseClass);
+E.setFileProximityDistance(Derived.FileProximityDistance);
+E.setSemaFileProximityScore(Relevance.SemaFileProximityScore);
+E.setSymbolScopeDistance(Derived.ScopeProximityDistance);
+E.setSemaSaysInScope(Relevance.SemaSaysInScope);
+E.setScope(Relevance.Scope);
+E.setContextKind(Relevance.Context);
+E.setIsInstanceMember(Relevance.IsInstanceMember);
+E.setHadContextType(Relevance.HadContextType);
+E.setHadSymbolType(Relevance.HadSymbolType);
+E.setTypeMatchesPreferred(Relevance.TypeMatchesPreferred);
+E.setFilterLength(Relevance.FilterLength);
+return Evaluate(E);
+  }
+}
+static void DexBuild(benchmark::State ) {
+  for (auto _ : State)
+RunDecisionForestPredicitons();
+}
+BENCHMARK(DexBuild);
+
+} // namespace
+} // namespace clangd
+} // namespace clang
+
+int main(int argc, char *argv[]) {
+  ::benchmark::Initialize(, argv);
+  ::benchmark::RunSpecifiedBenchmarks();
+}
Index: clang-tools-extra/clangd/benchmarks/CMakeLists.txt
===
--- clang-tools-extra/clangd/benchmarks/CMakeLists.txt
+++ clang-tools-extra/clangd/benchmarks/CMakeLists.txt
@@ -7,3 +7,11 @@
   clangDaemon
   LLVMSupport
   )
+
+add_benchmark(DecisionForestBenchmark DecisionForestBenchmark.cpp)
+
+target_link_libraries(DecisionForestBenchmark
+  PRIVATE
+  clangDaemon
+  LLVMSupport
+  )
\ No newline at end of file


Index: clang-tools-extra/clangd/benchmarks/DecisionForestBenchmark.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/benchmarks/DecisionForestBenchmark.cpp
@@ -0,0 +1,59 @@
+//===--- IndexBenchmark.cpp - Clangd index benchmarks ---*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "CompletionModel.h"
+#include "benchmark/benchmark.h"
+
+const char *IndexFilename;
+const char *RequestsFilename;
+
+namespace clang {
+namespace clangd {
+namespace {
+void RunDecisionForestPredicitons() {
+  constexpr int NumExperiments = 1;
+  for (int I = 0; I < NumExperiments; ++I) {
+Example E;
+
+E.setIsDeprecated(Quality.Deprecated);
+E.setIsReservedName(Quality.ReservedName);
+E.setIsImplementationDetail(Quality.ImplementationDetail);
+E.setNumReferences(Quality.References);
+E.setSymbolCategory(Quality.Category);
+
+E.setIsNameInContext(Derived.NameMatchesContext);
+E.setIsForbidden(Relevance.Forbidden);
+E.setIsInBaseClass(Relevance.InBaseClass);
+E.setFileProximityDistance(Derived.FileProximityDistance);
+E.setSemaFileProximityScore(Relevance.SemaFileProximityScore);
+E.setSymbolScopeDistance(Derived.ScopeProximityDistance);
+E.setSemaSaysInScope(Relevance.SemaSaysInScope);
+E.setScope(Relevance.Scope);
+E.setContextKind(Relevance.Context);
+E.setIsInstanceMember(Relevance.IsInstanceMember);
+E.setHadContextType(Relevance.HadContextType);
+

[PATCH] D88557: [HIP] Add option --gpu-instrument-lib=

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

In D88557#2303891 , @tra wrote:

> Perhaps we should start thinking of shipping some of that bitcode along with 
> clang. 
> Then the instrumentation library could be linked with automatically by the 
> driver when `-finstrument` is specified.
> We already need bitcode for the math library for both NVPTX and AMDGPU and 
> will likely need more for other things that depend on things that are 
> standard on the host.

Right


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

https://reviews.llvm.org/D88557

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

[PATCH] D86743: [analyzer] Ignore VLASizeChecker case that could cause crash

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

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

In D86743#2303922 , @NoQ wrote:

> One slightly redeeming thing about this crash is that it's assertion-only. 
> When built without assertions clang doesn't crash and this patch doesn't 
> really change its behavior (adding transition to a null state is equivalent 
> to adding no transitions at all). This means that the assertion did its job 
> and notified us of the serious issue but simply removing the assertion 
> doesn't bring *that* much benefit and we can probably afford to wait for a 
> more solid fix.

Yes, that is right. `CheckerContext::addTransition()` accepts `nullptr`, thus 
with assertions disabled it does exactly what this patch does. Thus I think 
that this patch could be abandoned now, and if the bug is not exactly the same 
as 28450  then a new bug report 
should be filed in //BugZilla//. And maybe the review on D69726 
 could be continued if that is part of the 
proper solution you suggested.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86743

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

[PATCH] D88019: [analyzer][solver] Fix issue with symbol non-equality tracking

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

steakhal added a comment.

In D88019#2303078 , @martong wrote:

> I like the second approach, i.e. to have a debug checker. But I don't see, 
> how would this checker validate all constraints at the moment when they are 
> added to the State.

`ProgramStateRef evalAssume(ProgramStateRef State, SVal Cond, bool Assumption)` 
checker callback is called **after** any assume call.
So, we would have a `State` which has all? the constraints stored in the 
appropriate GDM. I'm not sure if we should aggregate all the constraints till 
this node - like we do for refutation. I think, in this case, we should just 
make sure that the **current** state does not have any contradiction.

---

Here is my proof-of-concept:

I've  dumped the state and args of evalAssume for the repro example, pretending 
that we don't reach a code-path on the infeasable path to crash:

  void avoidInfeasibleConstraintforGT(int a, int b) {
int c = b - a;
if (c <= 0)
  return;
if (a != b) {
  clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
  return;
}
clang_analyzer_warnIfReached(); // eh, we will reach this..., #1
// These are commented out, to pretend that we don't find out that we are 
on an infeasable path...
// a == b
// if (c < 0)
//   ;
  }

We reach the `#1` line, but more importantly, we will have the following state 
dumps:

  evalAssume: assuming Cond: (reg_$1) != (reg_$0) to be true in 
state:
  "program_state": {
"constraints": [
  { "symbol": "(reg_$0) - (reg_$1)", "range": "{ [1, 
2147483647] }" },
  { "symbol": "(reg_$1) != (reg_$0)", "range": "{ 
[-2147483648, -1], [1, 2147483647] }" }
]
  }
  
  evalAssume: assuming Cond: (reg_$1) != (reg_$0) to be false in 
state:
  "program_state": {
"constraints": [
  { "symbol": "(reg_$0) - (reg_$1)", "range": "{ [1, 
2147483647] }" },
  { "symbol": "(reg_$1) != (reg_$0)", "range": "{ [0, 0] }" }
]
  }

As you can see, the **latter** is the //infeasable// path, and if we have had 
serialized the constraints and let the Z3 check it, we would have gotten that 
it's unfeasable.
At this point the checker can dump any useful data for debugging, and crash the 
analyzer to let us know that something really bad happened.

> [...] And that might be too slow, it would have the same speed as using z3 
> instead of the range based solver.

Yes, it will probably freaking slow, but at least we have something.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88019

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

[PATCH] D86743: [analyzer] Ignore VLASizeChecker case that could cause crash

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

> Yup, that's pretty bad.

One slightly redeeming thing about this crash is that it's assertion-only. When 
built without assertions clang doesn't crash and this patch doesn't really 
change its behavior (adding transition to a null state is equivalent to adding 
no transitions at all). This means that the assertion did its job and notified 
us of the serious issue but simply removing the assertion doesn't bring *that* 
much benefit and we can probably afford to wait for a more solid fix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86743

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

[PATCH] D86743: [analyzer] Ignore VLASizeChecker case that could cause crash

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

baloghadamsoftware added a comment.

@vabridgers Please try to apply D69726  and 
check whether it solves this crash!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86743

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

[PATCH] D86743: [analyzer] Ignore VLASizeChecker case that could cause crash

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

baloghadamsoftware added a comment.

In D86743#2303910 , @NoQ wrote:

>> The last comment for that bug is D69726 , 
>> but the bug is not closed to it seems to me that D69726 
>>  does not solve it, just takes a single 
>> step towards the solution.
>
> It might as well be that the patch isn't, well, committed.

Oh! I mislooked that. I saw `DynamicSize.cpp` already in the repository, but 
that is a prerequisite, not that one. Sorry!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86743

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

[PATCH] D86743: [analyzer] Ignore VLASizeChecker case that could cause crash

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

> The last comment for that bug is D69726 , 
> but the bug is not closed to it seems to me that D69726 
>  does not solve it, just takes a single step 
> towards the solution.

It might as well be that the patch isn't, well, committed.

> However, this one is a core checker, and not even alpha.

Yup, that's pretty bad.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86743

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

[PATCH] D88370: Emit predefined macro for wavefront size for amdgcn

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl updated this revision to Diff 295326.
yaxunl added a comment.

simplifies wavefrontsize64 target feature


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

https://reviews.llvm.org/D88370

Files:
  clang/lib/Basic/Targets/AMDGPU.cpp
  clang/lib/Basic/Targets/AMDGPU.h
  clang/lib/Driver/ToolChains/AMDGPU.cpp
  clang/test/Driver/amdgpu-features.c
  clang/test/Driver/amdgpu-macros.cl
  clang/test/Driver/hip-macros.hip
  clang/test/Driver/hip-toolchain-features.hip

Index: clang/test/Driver/hip-toolchain-features.hip
===
--- clang/test/Driver/hip-toolchain-features.hip
+++ clang/test/Driver/hip-toolchain-features.hip
@@ -37,8 +37,17 @@
 // RUN:   -mcumode -mcumode -mno-cumode -mwavefrontsize64 -mcumode \
 // RUN:   -mwavefrontsize64 -mno-wavefrontsize64 2>&1 \
 // RUN:   | FileCheck %s -check-prefix=DUP
-// DUP: {{.*}}clang{{.*}} "-target-feature" "-wavefrontsize16"
-// DUP-SAME: "-target-feature" "+wavefrontsize32"
-// DUP-SAME: "-target-feature" "-wavefrontsize64"
-// DUP-SAME: "-target-feature" "+cumode"
-// DUP: {{.*}}lld{{.*}} "-plugin-opt=-mattr=-wavefrontsize16,+wavefrontsize32,-wavefrontsize64,+cumode"
+// DUP: {{.*}}clang{{.*}} "-target-feature" "+cumode"
+// DUP-NOT: "-target-feature" "{{.*}}wavefrontsize16"
+// DUP-NOT: "-target-feature" "{{.*}}wavefrontsize32"
+// DUP-NOT: "-target-feature" "{{.*}}wavefrontsize64"
+// DUP: {{.*}}lld{{.*}} "-plugin-opt=-mattr=+cumode"
+
+// RUN: %clang -### -target x86_64-linux-gnu -fgpu-rdc -nogpulib \
+// RUN:   --cuda-gpu-arch=gfx1010 %s \
+// RUN:   -mno-wavefrontsize64 -mwavefrontsize64 2>&1 \
+// RUN:   | FileCheck %s -check-prefix=WAVE64
+// WAVE64: {{.*}}clang{{.*}} "-target-feature" "+wavefrontsize64"
+// WAVE64-NOT: "-target-feature" "{{.*}}wavefrontsize16"
+// WAVE64-NOT: "-target-feature" "{{.*}}wavefrontsize32"
+// WAVE64: {{.*}}lld{{.*}} "-plugin-opt=-mattr=+wavefrontsize64"
Index: clang/test/Driver/hip-macros.hip
===
--- /dev/null
+++ clang/test/Driver/hip-macros.hip
@@ -0,0 +1,20 @@
+// RUN: %clang -E -dM --offload-arch=gfx906 -mwavefrontsize64 \
+// RUN:   --cuda-device-only -nogpuinc -nogpulib \
+// RUN:   %s 2>&1 | FileCheck --check-prefixes=WAVE64 %s
+// RUN: %clang -E -dM --offload-arch=gfx1010 -mwavefrontsize64 \
+// RUN:   --cuda-device-only -nogpuinc -nogpulib \
+// RUN:   %s 2>&1 | FileCheck --check-prefixes=WAVE64 %s
+// RUN: %clang -E -dM --offload-arch=gfx906 -mwavefrontsize64 \
+// RUN:   --cuda-device-only -nogpuinc -nogpulib \
+// RUN:   -mno-wavefrontsize64 %s 2>&1 | FileCheck --check-prefixes=WAVE64 %s
+// RUN: %clang -E -dM --offload-arch=gfx1010 -mwavefrontsize64 \
+// RUN:   --cuda-device-only -nogpuinc -nogpulib \
+// RUN:   -mno-wavefrontsize64 %s 2>&1 | FileCheck --check-prefixes=WAVE32 %s
+// RUN: %clang -E -dM --offload-arch=gfx906 -mno-wavefrontsize64 \
+// RUN:   --cuda-device-only -nogpuinc -nogpulib \
+// RUN:   -mwavefrontsize64 %s 2>&1 | FileCheck --check-prefixes=WAVE64 %s
+// RUN: %clang -E -dM --offload-arch=gfx1010 -mno-wavefrontsize64 \
+// RUN:   --cuda-device-only -nogpuinc -nogpulib \
+// RUN:   -mwavefrontsize64 %s 2>&1 | FileCheck --check-prefixes=WAVE64 %s
+// WAVE64-DAG: #define __AMDGCN_WAVEFRONT_SIZE 64
+// WAVE32-DAG: #define __AMDGCN_WAVEFRONT_SIZE 32
Index: clang/test/Driver/amdgpu-macros.cl
===
--- clang/test/Driver/amdgpu-macros.cl
+++ clang/test/Driver/amdgpu-macros.cl
@@ -346,4 +346,42 @@
 // GFX1011-DAG: #define __amdgcn_processor__ "gfx1011"
 // GFX1012-DAG: #define __amdgcn_processor__ "gfx1012"
 // GFX1030-DAG: #define __amdgcn_processor__ "gfx1030"
-// GFX1031-DAG: #define __amdgcn_processor__ "gfx1031"
\ No newline at end of file
+// GFX1031-DAG: #define __amdgcn_processor__ "gfx1031"
+
+// GFX600-DAG: #define __AMDGCN_WAVEFRONT_SIZE 64
+// GFX601-DAG: #define __AMDGCN_WAVEFRONT_SIZE 64
+// GFX700-DAG: #define __AMDGCN_WAVEFRONT_SIZE 64
+// GFX701-DAG: #define __AMDGCN_WAVEFRONT_SIZE 64
+// GFX702-DAG: #define __AMDGCN_WAVEFRONT_SIZE 64
+// GFX703-DAG: #define __AMDGCN_WAVEFRONT_SIZE 64
+// GFX704-DAG: #define __AMDGCN_WAVEFRONT_SIZE 64
+// GFX801-DAG: #define __AMDGCN_WAVEFRONT_SIZE 64
+// GFX802-DAG: #define __AMDGCN_WAVEFRONT_SIZE 64
+// GFX803-DAG: #define __AMDGCN_WAVEFRONT_SIZE 64
+// GFX810-DAG: #define __AMDGCN_WAVEFRONT_SIZE 64
+// GFX900-DAG: #define __AMDGCN_WAVEFRONT_SIZE 64
+// GFX902-DAG: #define __AMDGCN_WAVEFRONT_SIZE 64
+// GFX904-DAG: #define __AMDGCN_WAVEFRONT_SIZE 64
+// GFX906-DAG: #define __AMDGCN_WAVEFRONT_SIZE 64
+// GFX908-DAG: #define __AMDGCN_WAVEFRONT_SIZE 64
+// GFX909-DAG: #define __AMDGCN_WAVEFRONT_SIZE 64
+// GFX1010-DAG: #define __AMDGCN_WAVEFRONT_SIZE 32
+// GFX1011-DAG: #define __AMDGCN_WAVEFRONT_SIZE 32
+// GFX1012-DAG: #define __AMDGCN_WAVEFRONT_SIZE 32
+// GFX1030-DAG: #define __AMDGCN_WAVEFRONT_SIZE 32
+// GFX1031-DAG: #define 

[PATCH] D88557: [HIP] Add option --gpu-instrument-lib=

[Artem Belevich via Phabricator via cfe-commits]

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

Perhaps we should start thinking of shipping some of that bitcode along with 
clang. 
Then the instrumentation library could be linked with automatically by the 
driver when `-finstrument` is specified.
We already need bitcode for the math library for both NVPTX and AMDGPU and will 
likely need more for other things that depend on things that are standard on 
the host.


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

https://reviews.llvm.org/D88557

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

[PATCH] D69726: [analyzer] DynamicSize: Store the dynamic size

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Thank you for reminding me of this patch. I still think it's a pretty important 
patch and i'm interested in getting it landed.




Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:1388
- ->castAs()
- ->StripCasts()
- ->castAs();

What happened to this `StripCasts()`? I don't see it in the after-code and I 
vaguely remember having to be very careful with it.


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

https://reviews.llvm.org/D69726

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

[PATCH] D88265: Fix comma with half vectors.

[Alexandre Rames via Phabricator via cfe-commits]

arames added a comment.

In D88265#2302653 , @fhahn wrote:

> Could you adjust the commit message to be a bit more descriptive, e.g 
> something like `[Sema] Support Comma operator for fp16 vectors.`

Done!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88265

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

[PATCH] D88265: Fix comma with half vectors.

[Alexandre Rames via Phabricator via cfe-commits]

arames updated this revision to Diff 295316.
arames added a comment.

Update commit message.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88265

Files:
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/fp16vec-sema.c


Index: clang/test/Sema/fp16vec-sema.c
===
--- clang/test/Sema/fp16vec-sema.c
+++ clang/test/Sema/fp16vec-sema.c
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wno-unused-value -verify %s
 
 typedef __fp16 half4 __attribute__ ((vector_size (8)));
 typedef float float4 __attribute__ ((vector_size (16)));
@@ -28,6 +28,8 @@
   sv0 = hv0 >= hv1;
   sv0 = hv0 || hv1; // expected-error{{logical expression with vector types 
'half4' (vector of 4 '__fp16' values) and 'half4' is only supported in C++}}
   sv0 = hv0 && hv1; // expected-error{{logical expression with vector types 
'half4' (vector of 4 '__fp16' values) and 'half4' is only supported in C++}}
+  hv0, 1;
+  1, hv0;
 
   // Implicit conversion between half vectors and float vectors are not 
allowed.
   hv0 = fv0; // expected-error{{assigning to}}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -13933,9 +13933,10 @@
   // float vectors and truncating the result back to half vector. For now, we 
do
   // this only when HalfArgsAndReturn is set (that is, when the target is arm 
or
   // arm64).
-  assert(isVector(RHS.get()->getType(), Context.HalfTy) ==
- isVector(LHS.get()->getType(), Context.HalfTy) &&
- "both sides are half vectors or neither sides are");
+  assert(
+  (Opc == BO_Comma || isVector(RHS.get()->getType(), Context.HalfTy) ==
+  isVector(LHS.get()->getType(), Context.HalfTy)) 
&&
+  "both sides are half vectors or neither sides are");
   ConvertHalfVec =
   needsConversionOfHalfVec(ConvertHalfVec, Context, LHS.get(), RHS.get());
 


Index: clang/test/Sema/fp16vec-sema.c
===
--- clang/test/Sema/fp16vec-sema.c
+++ clang/test/Sema/fp16vec-sema.c
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wno-unused-value -verify %s
 
 typedef __fp16 half4 __attribute__ ((vector_size (8)));
 typedef float float4 __attribute__ ((vector_size (16)));
@@ -28,6 +28,8 @@
   sv0 = hv0 >= hv1;
   sv0 = hv0 || hv1; // expected-error{{logical expression with vector types 'half4' (vector of 4 '__fp16' values) and 'half4' is only supported in C++}}
   sv0 = hv0 && hv1; // expected-error{{logical expression with vector types 'half4' (vector of 4 '__fp16' values) and 'half4' is only supported in C++}}
+  hv0, 1;
+  1, hv0;
 
   // Implicit conversion between half vectors and float vectors are not allowed.
   hv0 = fv0; // expected-error{{assigning to}}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -13933,9 +13933,10 @@
   // float vectors and truncating the result back to half vector. For now, we do
   // this only when HalfArgsAndReturn is set (that is, when the target is arm or
   // arm64).
-  assert(isVector(RHS.get()->getType(), Context.HalfTy) ==
- isVector(LHS.get()->getType(), Context.HalfTy) &&
- "both sides are half vectors or neither sides are");
+  assert(
+  (Opc == BO_Comma || isVector(RHS.get()->getType(), Context.HalfTy) ==
+  isVector(LHS.get()->getType(), Context.HalfTy)) &&
+  "both sides are half vectors or neither sides are");
   ConvertHalfVec =
   needsConversionOfHalfVec(ConvertHalfVec, Context, LHS.get(), RHS.get());
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D88370: Emit predefined macro for wavefront size for amdgcn

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl updated this revision to Diff 295312.
yaxunl added a comment.

simpler code for handling multiple wave64 options


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

https://reviews.llvm.org/D88370

Files:
  clang/lib/Basic/Targets/AMDGPU.cpp
  clang/lib/Basic/Targets/AMDGPU.h
  clang/lib/Driver/ToolChains/AMDGPU.cpp
  clang/test/Driver/amdgpu-features.c
  clang/test/Driver/amdgpu-macros.cl
  clang/test/Driver/hip-macros.hip
  clang/test/Driver/hip-toolchain-features.hip

Index: clang/test/Driver/hip-toolchain-features.hip
===
--- clang/test/Driver/hip-toolchain-features.hip
+++ clang/test/Driver/hip-toolchain-features.hip
@@ -42,3 +42,12 @@
 // DUP-SAME: "-target-feature" "-wavefrontsize64"
 // DUP-SAME: "-target-feature" "+cumode"
 // DUP: {{.*}}lld{{.*}} "-plugin-opt=-mattr=-wavefrontsize16,+wavefrontsize32,-wavefrontsize64,+cumode"
+
+// RUN: %clang -### -target x86_64-linux-gnu -fgpu-rdc -nogpulib \
+// RUN:   --cuda-gpu-arch=gfx1010 %s \
+// RUN:   -mno-wavefrontsize64 -mwavefrontsize64 2>&1 \
+// RUN:   | FileCheck %s -check-prefix=WAVE64
+// WAVE64: {{.*}}clang{{.*}} "-target-feature" "-wavefrontsize16"
+// WAVE64-SAME: "-target-feature" "-wavefrontsize32"
+// WAVE64-SAME: "-target-feature" "+wavefrontsize64"
+// WAVE64: {{.*}}lld{{.*}} "-plugin-opt=-mattr=-wavefrontsize16,-wavefrontsize32,+wavefrontsize64"
Index: clang/test/Driver/hip-macros.hip
===
--- /dev/null
+++ clang/test/Driver/hip-macros.hip
@@ -0,0 +1,20 @@
+// RUN: %clang -E -dM --offload-arch=gfx906 -mwavefrontsize64 \
+// RUN:   --cuda-device-only -nogpuinc -nogpulib \
+// RUN:   %s 2>&1 | FileCheck --check-prefixes=WAVE64 %s
+// RUN: %clang -E -dM --offload-arch=gfx1010 -mwavefrontsize64 \
+// RUN:   --cuda-device-only -nogpuinc -nogpulib \
+// RUN:   %s 2>&1 | FileCheck --check-prefixes=WAVE64 %s
+// RUN: %clang -E -dM --offload-arch=gfx906 -mwavefrontsize64 \
+// RUN:   --cuda-device-only -nogpuinc -nogpulib \
+// RUN:   -mno-wavefrontsize64 %s 2>&1 | FileCheck --check-prefixes=WAVE64 %s
+// RUN: %clang -E -dM --offload-arch=gfx1010 -mwavefrontsize64 \
+// RUN:   --cuda-device-only -nogpuinc -nogpulib \
+// RUN:   -mno-wavefrontsize64 %s 2>&1 | FileCheck --check-prefixes=WAVE32 %s
+// RUN: %clang -E -dM --offload-arch=gfx906 -mno-wavefrontsize64 \
+// RUN:   --cuda-device-only -nogpuinc -nogpulib \
+// RUN:   -mwavefrontsize64 %s 2>&1 | FileCheck --check-prefixes=WAVE64 %s
+// RUN: %clang -E -dM --offload-arch=gfx1010 -mno-wavefrontsize64 \
+// RUN:   --cuda-device-only -nogpuinc -nogpulib \
+// RUN:   -mwavefrontsize64 %s 2>&1 | FileCheck --check-prefixes=WAVE64 %s
+// WAVE64-DAG: #define __AMDGCN_WAVEFRONT_SIZE 64
+// WAVE32-DAG: #define __AMDGCN_WAVEFRONT_SIZE 32
Index: clang/test/Driver/amdgpu-macros.cl
===
--- clang/test/Driver/amdgpu-macros.cl
+++ clang/test/Driver/amdgpu-macros.cl
@@ -346,4 +346,42 @@
 // GFX1011-DAG: #define __amdgcn_processor__ "gfx1011"
 // GFX1012-DAG: #define __amdgcn_processor__ "gfx1012"
 // GFX1030-DAG: #define __amdgcn_processor__ "gfx1030"
-// GFX1031-DAG: #define __amdgcn_processor__ "gfx1031"
\ No newline at end of file
+// GFX1031-DAG: #define __amdgcn_processor__ "gfx1031"
+
+// GFX600-DAG: #define __AMDGCN_WAVEFRONT_SIZE 64
+// GFX601-DAG: #define __AMDGCN_WAVEFRONT_SIZE 64
+// GFX700-DAG: #define __AMDGCN_WAVEFRONT_SIZE 64
+// GFX701-DAG: #define __AMDGCN_WAVEFRONT_SIZE 64
+// GFX702-DAG: #define __AMDGCN_WAVEFRONT_SIZE 64
+// GFX703-DAG: #define __AMDGCN_WAVEFRONT_SIZE 64
+// GFX704-DAG: #define __AMDGCN_WAVEFRONT_SIZE 64
+// GFX801-DAG: #define __AMDGCN_WAVEFRONT_SIZE 64
+// GFX802-DAG: #define __AMDGCN_WAVEFRONT_SIZE 64
+// GFX803-DAG: #define __AMDGCN_WAVEFRONT_SIZE 64
+// GFX810-DAG: #define __AMDGCN_WAVEFRONT_SIZE 64
+// GFX900-DAG: #define __AMDGCN_WAVEFRONT_SIZE 64
+// GFX902-DAG: #define __AMDGCN_WAVEFRONT_SIZE 64
+// GFX904-DAG: #define __AMDGCN_WAVEFRONT_SIZE 64
+// GFX906-DAG: #define __AMDGCN_WAVEFRONT_SIZE 64
+// GFX908-DAG: #define __AMDGCN_WAVEFRONT_SIZE 64
+// GFX909-DAG: #define __AMDGCN_WAVEFRONT_SIZE 64
+// GFX1010-DAG: #define __AMDGCN_WAVEFRONT_SIZE 32
+// GFX1011-DAG: #define __AMDGCN_WAVEFRONT_SIZE 32
+// GFX1012-DAG: #define __AMDGCN_WAVEFRONT_SIZE 32
+// GFX1030-DAG: #define __AMDGCN_WAVEFRONT_SIZE 32
+// GFX1031-DAG: #define __AMDGCN_WAVEFRONT_SIZE 32
+
+// RUN: %clang -E -dM -target amdgcn -mcpu=gfx906 -mwavefrontsize64 \
+// RUN:   %s 2>&1 | FileCheck --check-prefixes=WAVE64 %s
+// RUN: %clang -E -dM -target amdgcn -mcpu=gfx1010 -mwavefrontsize64 \
+// RUN:   %s 2>&1 | FileCheck --check-prefixes=WAVE64 %s
+// RUN: %clang -E -dM -target amdgcn -mcpu=gfx906 -mwavefrontsize64 \
+// RUN:   -mno-wavefrontsize64 %s 2>&1 | FileCheck --check-prefixes=WAVE64 %s
+// RUN: %clang -E -dM -target amdgcn -mcpu=gfx1010 -mwavefrontsize64 \
+// RUN:   -mno-wavefrontsize64 

[PATCH] D88370: Emit predefined macro for wavefront size for amdgcn

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added inline comments.



Comment at: clang/lib/Driver/ToolChains/AMDGPU.cpp:394-395
+  // Get the last argument of -mwavefrontsize64 or -mno-wavefrontsize64.
+  for (auto WaveArg : Args.filtered_reverse(options::OPT_mwavefrontsize64,
+options::OPT_mno_wavefrontsize64)) 
{
+if (WaveArg->getOption().getID() == options::OPT_mwavefrontsize64) {

arsenm wrote:
> yaxunl wrote:
> > arsenm wrote:
> > > Why isn't this using hasFlag?
> > > e.g. like
> > > 
> > > ```
> > > DriverArgs.hasFlag(options::OPT_fcuda_flush_denormals_to_zero,
> > >options::OPT_fno_cuda_flush_denormals_to_zero,
> > >getDefaultDenormsAreZeroForTarget(Kind)))
> > > ```
> > hasFlag always return true or false, but here we have 3 cases : no arg, 
> > last arg is wave64, last arg is no-wave64.
> hasFlag considers all of these. The features only really need to add 
> +wavefrontsze64 or not based on true or false, the rest is just noise
I have a simpler solution. will update


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

https://reviews.llvm.org/D88370

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

[PATCH] D88370: Emit predefined macro for wavefront size for amdgcn

[Matt Arsenault via Phabricator via cfe-commits]

arsenm added inline comments.



Comment at: clang/lib/Driver/ToolChains/AMDGPU.cpp:394-395
+  // Get the last argument of -mwavefrontsize64 or -mno-wavefrontsize64.
+  for (auto WaveArg : Args.filtered_reverse(options::OPT_mwavefrontsize64,
+options::OPT_mno_wavefrontsize64)) 
{
+if (WaveArg->getOption().getID() == options::OPT_mwavefrontsize64) {

yaxunl wrote:
> arsenm wrote:
> > Why isn't this using hasFlag?
> > e.g. like
> > 
> > ```
> > DriverArgs.hasFlag(options::OPT_fcuda_flush_denormals_to_zero,
> >options::OPT_fno_cuda_flush_denormals_to_zero,
> >getDefaultDenormsAreZeroForTarget(Kind)))
> > ```
> hasFlag always return true or false, but here we have 3 cases : no arg, last 
> arg is wave64, last arg is no-wave64.
hasFlag considers all of these. The features only really need to add 
+wavefrontsze64 or not based on true or false, the rest is just noise


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

https://reviews.llvm.org/D88370

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

[PATCH] D88370: Emit predefined macro for wavefront size for amdgcn

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added inline comments.



Comment at: clang/lib/Driver/ToolChains/AMDGPU.cpp:394-395
+  // Get the last argument of -mwavefrontsize64 or -mno-wavefrontsize64.
+  for (auto WaveArg : Args.filtered_reverse(options::OPT_mwavefrontsize64,
+options::OPT_mno_wavefrontsize64)) 
{
+if (WaveArg->getOption().getID() == options::OPT_mwavefrontsize64) {

arsenm wrote:
> Why isn't this using hasFlag?
> e.g. like
> 
> ```
> DriverArgs.hasFlag(options::OPT_fcuda_flush_denormals_to_zero,
>options::OPT_fno_cuda_flush_denormals_to_zero,
>getDefaultDenormsAreZeroForTarget(Kind)))
> ```
hasFlag always return true or false, but here we have 3 cases : no arg, last 
arg is wave64, last arg is no-wave64.


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

https://reviews.llvm.org/D88370

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

[PATCH] D88370: Emit predefined macro for wavefront size for amdgcn

[Matt Arsenault via Phabricator via cfe-commits]

arsenm added inline comments.



Comment at: clang/lib/Driver/ToolChains/AMDGPU.cpp:394-395
+  // Get the last argument of -mwavefrontsize64 or -mno-wavefrontsize64.
+  for (auto WaveArg : Args.filtered_reverse(options::OPT_mwavefrontsize64,
+options::OPT_mno_wavefrontsize64)) 
{
+if (WaveArg->getOption().getID() == options::OPT_mwavefrontsize64) {

Why isn't this using hasFlag?
e.g. like

```
DriverArgs.hasFlag(options::OPT_fcuda_flush_denormals_to_zero,
   options::OPT_fno_cuda_flush_denormals_to_zero,
   getDefaultDenormsAreZeroForTarget(Kind)))
```


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

https://reviews.llvm.org/D88370

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

[PATCH] D88370: Emit predefined macro for wavefront size for amdgcn

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl updated this revision to Diff 295297.
yaxunl edited the summary of this revision.
yaxunl added a comment.

Add test and fix multiple -m[no-]wavefrontsize64 issue.


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

https://reviews.llvm.org/D88370

Files:
  clang/lib/Basic/Targets/AMDGPU.cpp
  clang/lib/Basic/Targets/AMDGPU.h
  clang/lib/Driver/ToolChains/AMDGPU.cpp
  clang/test/Driver/amdgpu-features.c
  clang/test/Driver/amdgpu-macros.cl
  clang/test/Driver/hip-macros.hip
  clang/test/Driver/hip-toolchain-features.hip

Index: clang/test/Driver/hip-toolchain-features.hip
===
--- clang/test/Driver/hip-toolchain-features.hip
+++ clang/test/Driver/hip-toolchain-features.hip
@@ -42,3 +42,12 @@
 // DUP-SAME: "-target-feature" "-wavefrontsize64"
 // DUP-SAME: "-target-feature" "+cumode"
 // DUP: {{.*}}lld{{.*}} "-plugin-opt=-mattr=-wavefrontsize16,+wavefrontsize32,-wavefrontsize64,+cumode"
+
+// RUN: %clang -### -target x86_64-linux-gnu -fgpu-rdc -nogpulib \
+// RUN:   --cuda-gpu-arch=gfx1010 %s \
+// RUN:   -mno-wavefrontsize64 -mwavefrontsize64 2>&1 \
+// RUN:   | FileCheck %s -check-prefix=WAVE64
+// WAVE64: {{.*}}clang{{.*}} "-target-feature" "-wavefrontsize16"
+// WAVE64-SAME: "-target-feature" "-wavefrontsize32"
+// WAVE64-SAME: "-target-feature" "+wavefrontsize64"
+// WAVE64: {{.*}}lld{{.*}} "-plugin-opt=-mattr=-wavefrontsize16,-wavefrontsize32,+wavefrontsize64"
Index: clang/test/Driver/hip-macros.hip
===
--- /dev/null
+++ clang/test/Driver/hip-macros.hip
@@ -0,0 +1,20 @@
+// RUN: %clang -E -dM --offload-arch=gfx906 -mwavefrontsize64 \
+// RUN:   --cuda-device-only -nogpuinc -nogpulib \
+// RUN:   %s 2>&1 | FileCheck --check-prefixes=WAVE64 %s
+// RUN: %clang -E -dM --offload-arch=gfx1010 -mwavefrontsize64 \
+// RUN:   --cuda-device-only -nogpuinc -nogpulib \
+// RUN:   %s 2>&1 | FileCheck --check-prefixes=WAVE64 %s
+// RUN: %clang -E -dM --offload-arch=gfx906 -mwavefrontsize64 \
+// RUN:   --cuda-device-only -nogpuinc -nogpulib \
+// RUN:   -mno-wavefrontsize64 %s 2>&1 | FileCheck --check-prefixes=WAVE64 %s
+// RUN: %clang -E -dM --offload-arch=gfx1010 -mwavefrontsize64 \
+// RUN:   --cuda-device-only -nogpuinc -nogpulib \
+// RUN:   -mno-wavefrontsize64 %s 2>&1 | FileCheck --check-prefixes=WAVE32 %s
+// RUN: %clang -E -dM --offload-arch=gfx906 -mno-wavefrontsize64 \
+// RUN:   --cuda-device-only -nogpuinc -nogpulib \
+// RUN:   -mwavefrontsize64 %s 2>&1 | FileCheck --check-prefixes=WAVE64 %s
+// RUN: %clang -E -dM --offload-arch=gfx1010 -mno-wavefrontsize64 \
+// RUN:   --cuda-device-only -nogpuinc -nogpulib \
+// RUN:   -mwavefrontsize64 %s 2>&1 | FileCheck --check-prefixes=WAVE64 %s
+// WAVE64-DAG: #define __AMDGCN_WAVEFRONT_SIZE 64
+// WAVE32-DAG: #define __AMDGCN_WAVEFRONT_SIZE 32
Index: clang/test/Driver/amdgpu-macros.cl
===
--- clang/test/Driver/amdgpu-macros.cl
+++ clang/test/Driver/amdgpu-macros.cl
@@ -346,4 +346,42 @@
 // GFX1011-DAG: #define __amdgcn_processor__ "gfx1011"
 // GFX1012-DAG: #define __amdgcn_processor__ "gfx1012"
 // GFX1030-DAG: #define __amdgcn_processor__ "gfx1030"
-// GFX1031-DAG: #define __amdgcn_processor__ "gfx1031"
\ No newline at end of file
+// GFX1031-DAG: #define __amdgcn_processor__ "gfx1031"
+
+// GFX600-DAG: #define __AMDGCN_WAVEFRONT_SIZE 64
+// GFX601-DAG: #define __AMDGCN_WAVEFRONT_SIZE 64
+// GFX700-DAG: #define __AMDGCN_WAVEFRONT_SIZE 64
+// GFX701-DAG: #define __AMDGCN_WAVEFRONT_SIZE 64
+// GFX702-DAG: #define __AMDGCN_WAVEFRONT_SIZE 64
+// GFX703-DAG: #define __AMDGCN_WAVEFRONT_SIZE 64
+// GFX704-DAG: #define __AMDGCN_WAVEFRONT_SIZE 64
+// GFX801-DAG: #define __AMDGCN_WAVEFRONT_SIZE 64
+// GFX802-DAG: #define __AMDGCN_WAVEFRONT_SIZE 64
+// GFX803-DAG: #define __AMDGCN_WAVEFRONT_SIZE 64
+// GFX810-DAG: #define __AMDGCN_WAVEFRONT_SIZE 64
+// GFX900-DAG: #define __AMDGCN_WAVEFRONT_SIZE 64
+// GFX902-DAG: #define __AMDGCN_WAVEFRONT_SIZE 64
+// GFX904-DAG: #define __AMDGCN_WAVEFRONT_SIZE 64
+// GFX906-DAG: #define __AMDGCN_WAVEFRONT_SIZE 64
+// GFX908-DAG: #define __AMDGCN_WAVEFRONT_SIZE 64
+// GFX909-DAG: #define __AMDGCN_WAVEFRONT_SIZE 64
+// GFX1010-DAG: #define __AMDGCN_WAVEFRONT_SIZE 32
+// GFX1011-DAG: #define __AMDGCN_WAVEFRONT_SIZE 32
+// GFX1012-DAG: #define __AMDGCN_WAVEFRONT_SIZE 32
+// GFX1030-DAG: #define __AMDGCN_WAVEFRONT_SIZE 32
+// GFX1031-DAG: #define __AMDGCN_WAVEFRONT_SIZE 32
+
+// RUN: %clang -E -dM -target amdgcn -mcpu=gfx906 -mwavefrontsize64 \
+// RUN:   %s 2>&1 | FileCheck --check-prefixes=WAVE64 %s
+// RUN: %clang -E -dM -target amdgcn -mcpu=gfx1010 -mwavefrontsize64 \
+// RUN:   %s 2>&1 | FileCheck --check-prefixes=WAVE64 %s
+// RUN: %clang -E -dM -target amdgcn -mcpu=gfx906 -mwavefrontsize64 \
+// RUN:   -mno-wavefrontsize64 %s 2>&1 | FileCheck --check-prefixes=WAVE64 %s
+// RUN: %clang -E -dM -target amdgcn -mcpu=gfx1010 

[PATCH] D88546: [ARM] Update NEON testcase with missing target string in

[John Brawn via Phabricator via cfe-commits]

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

LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88546

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

[PATCH] D88338: [clangd] clangd --check: standalone diagnosis of common problems

[Sam McCall via Phabricator via cfe-commits]

sammccall updated this revision to Diff 295290.
sammccall marked 6 inline comments as done.
sammccall added a comment.

address (some of) comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88338

Files:
  clang-tools-extra/clangd/test/check-fail.test
  clang-tools-extra/clangd/test/check.test
  clang-tools-extra/clangd/tool/CMakeLists.txt
  clang-tools-extra/clangd/tool/Check.cpp
  clang-tools-extra/clangd/tool/ClangdMain.cpp

Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -47,6 +47,11 @@
 
 namespace clang {
 namespace clangd {
+
+// Implemented in Check.cpp.
+bool check(const llvm::StringRef File, const ThreadsafeFS ,
+   const ClangdLSPServer::Options );
+
 namespace {
 
 using llvm::cl::cat;
@@ -57,6 +62,7 @@
 using llvm::cl::list;
 using llvm::cl::opt;
 using llvm::cl::OptionCategory;
+using llvm::cl::ValueOptional;
 using llvm::cl::values;
 
 // All flags must be placed in a category, or they will be shown neither in
@@ -354,6 +360,16 @@
 Hidden,
 };
 
+opt CheckFile{
+"check",
+cat(Misc),
+desc("Parse one file in isolation instead of acting as a language server. "
+ "Useful to investigate/reproduce crashes or configuration problems. "
+ "With --check=, attempts to parse a particular file."),
+init(""),
+ValueOptional,
+};
+
 enum PCHStorageFlag { Disk, Memory };
 opt PCHStorage{
 "pch-storage",
@@ -541,7 +557,8 @@
 
 enum class ErrorResultCode : int {
   NoShutdownRequest = 1,
-  CantRunAsXPCService = 2
+  CantRunAsXPCService = 2,
+  CheckFailed = 3
 };
 
 int main(int argc, char *argv[]) {
@@ -646,7 +663,8 @@
   // If a user ran `clangd` in a terminal without redirecting anything,
   // it's somewhat likely they're confused about how to use clangd.
   // Show them the help overview, which explains.
-  if (llvm::outs().is_displayed() && llvm::errs().is_displayed())
+  if (llvm::outs().is_displayed() && llvm::errs().is_displayed() &&
+  !CheckFile.getNumOccurrences())
 llvm::errs() << Overview << "\n";
   // Use buffered stream to stderr (we still flush each log message). Unbuffered
   // stream can cause significant (non-deterministic) latency for the logger.
@@ -825,6 +843,15 @@
   // Shall we allow to customize the file limit?
   Opts.Rename.AllowCrossFile = CrossFileRename;
 
+  if (CheckFile.getNumOccurrences()) {
+llvm::SmallString<256> Path;
+llvm::sys::fs::real_path(CheckFile, Path, /*expand_tilde=*/true);
+log("Entering check mode (no LSP server)");
+return check(Path, TFS, Opts)
+   ? 0
+   : static_cast(ErrorResultCode::CheckFailed);
+  }
+
   // Initialize and run ClangdLSPServer.
   // Change stdin to binary to not lose \r\n on windows.
   llvm::sys::ChangeStdinToBinary();
@@ -835,7 +862,7 @@
 TransportLayer = newXPCTransport();
 #else
 llvm::errs() << "This clangd binary wasn't built with XPC support.\n";
-return (int)ErrorResultCode::CantRunAsXPCService;
+return static_cast(ErrorResultCode::CantRunAsXPCService);
 #endif
   } else {
 log("Starting LSP over stdin/stdout");
Index: clang-tools-extra/clangd/tool/Check.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/tool/Check.cpp
@@ -0,0 +1,259 @@
+//===--- Check.cpp - clangd self-diagnostics --===//
+//
+// 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
+//
+//===--===//
+//
+// Many basic problems can occur processing a file in clangd, e.g.:
+//  - system includes are not found
+//  - crash when indexing its AST
+// clangd --check provides a simplified, isolated way to reproduce these,
+// with no editor, LSP, threads, background indexing etc to contend with.
+//
+// One important use case is gathering information for bug reports.
+// Another is reproducing crashes, and checking which setting prevent them.
+//
+// It simulates opening a file (determining compile command, parsing, indexing)
+// and then running features at many locations.
+//
+// Currently it adds some basic logging of progress and results.
+// We should consider extending it to also recognize common symptoms and
+// recommend solutions (e.g. standard library installation issues).
+//
+//===--===//
+
+#include "ClangdLSPServer.h"
+#include "CodeComplete.h"
+#include "GlobalCompilationDatabase.h"
+#include "Hover.h"
+#include "ParsedAST.h"
+#include "Preamble.h"
+#include "SourceCode.h"
+#include "XRefs.h"
+#include 

[PATCH] D88338: [clangd] clangd --check: standalone diagnosis of common problems

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: clang-tools-extra/clangd/tool/Check.cpp:87
+auto Mangler = CommandMangler::detect();
+// Check for missing resource dir?
+if (Opts.ResourceDir)

kadircet wrote:
> i agree, this would help identifying the case of "clangd binary has been 
> copied to some place without the necessary lib directory".
> 
> but i think we should check for the final `-resource-dir` in the compile 
> command below. as user's compile flags can override whatever default clangd 
> comes up with.
Moved FIXME to the right place.



Comment at: clang-tools-extra/clangd/tool/Check.cpp:125
+Inputs.Opts.ClangTidyOpts =
+Opts.GetClangTidyOptions(*TFS.view(llvm::None), File);
+log("Parsing command...");

kadircet wrote:
> IIRC, option providers don't really log anything about success/failure of 
> parsing the config file. what's the point of having this exactly?
This is needed in order to run the correct clang-tidy checks.



Comment at: clang-tools-extra/clangd/tool/Check.cpp:126
+Opts.GetClangTidyOptions(*TFS.view(llvm::None), File);
+log("Parsing command...");
+Invocation =

kadircet wrote:
> it is clear that we've crashed while parsing compile commands if we don't see 
> `cc1 args are` in the output. so maybe drop this one?
I don't think that's clear - you can find it out by reading the code, but I 
expect people to be running this who aren't reading the code. (They won't be 
able to work out all the details, but they tend to be interested in the broad 
strokes of what's going on).

So generally I've tried to include log statements before each major step.



Comment at: clang-tools-extra/clangd/tool/Check.cpp:131
+showErrors(InvocationDiags);
+log("cc1 args are: {0}", llvm::join(CC1Args, " "));
+if (!Invocation) {

kadircet wrote:
> maybe we should also note that this doesn't reflect the final result. as we 
> turn off pchs or dependency file outputting related flags afterwards.
Sure, and we also run clang-tidy, and fiddle with the preamble, and skip 
function bodies...

I think a disclaimer that "running clangd is not equivalent to running clang 
" would be somewhat confusing, as I'm not sure that's a thing that 
anyone would expect to be true.

I expect the cc1 args to be useless to anyone not familiar with clangd 
internals, but they're kind of important to include. Reworded the log message a 
bit, is this enough?



Comment at: clang-tools-extra/clangd/tool/Check.cpp:202
+  }
+  unsigned Definitions = locateSymbolAt(*AST, Pos, ).size();
+  vlog("definition: {0}", Definitions);

kadircet wrote:
> maybe we could print errors if the following has no results for "identifiers" 
> ?
There are lots of ways go-to-def can yield no results (e.g. macros, #ifdef-'d 
sections, templated code we can't resolve)



Comment at: clang-tools-extra/clangd/tool/ClangdMain.cpp:52
+// Implemented in Check.cpp.
+bool check(const llvm::StringRef File, const ThreadsafeFS ,
+   llvm::Optional CompileCommandsPath,

kadircet wrote:
> why not have this in `Check.h` ?
A header seems a bit out of place in tool/ and it doesn't seem necessary, being 
one function with a simple signature and no unit tests.

We'll get a linker error if we ever get this wrong, right?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88338

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

[clang] 3a7487f - [FE] Use preferred alignment instead of ABI alignment for complete object when applicable

[Xiangling Liao via cfe-commits]

Author: Xiangling Liao
Date: 2020-09-30T10:48:28-04:00
New Revision: 3a7487f903e2a6be29de39058eee2372e30798d5

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

LOG: [FE] Use preferred alignment instead of ABI alignment for complete object 
when applicable

On some targets, preferred alignment is larger than ABI alignment in some 
cases. For example,
on AIX we have special power alignment rules which would cause that. 
Previously, to support
those cases, we added a “PreferredAlignment” field in the `RecordLayout` to 
store the AIX
special alignment values in “PreferredAlignment” as the community suggested.

However, that patch alone is not enough. There are places in the Clang where 
`PreferredAlignment`
should have been used instead of ABI-specified alignment. This patch is aimed 
at fixing those
spots.

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

Added: 
clang/test/CodeGen/aix-alignment.c
clang/test/CodeGenCXX/aix-alignment.cpp

Modified: 
clang/include/clang/AST/ASTContext.h
clang/lib/AST/ASTContext.cpp
clang/lib/CodeGen/CGExprCXX.cpp
clang/lib/CodeGen/ItaniumCXXABI.cpp
clang/lib/CodeGen/TargetInfo.cpp

Removed: 




diff  --git a/clang/include/clang/AST/ASTContext.h 
b/clang/include/clang/AST/ASTContext.h
index de0d1198b6d4..d30cf045f104 100644
--- a/clang/include/clang/AST/ASTContext.h
+++ b/clang/include/clang/AST/ASTContext.h
@@ -2134,16 +2134,25 @@ class ASTContext : public RefCountedBase {
   }
   unsigned getTypeUnadjustedAlign(const Type *T) const;
 
-  /// Return the ABI-specified alignment of a type, in bits, or 0 if
+  /// Return the alignment of a type, in bits, or 0 if
   /// the type is incomplete and we cannot determine the alignment (for
-  /// example, from alignment attributes).
-  unsigned getTypeAlignIfKnown(QualType T) const;
+  /// example, from alignment attributes). The returned alignment is the
+  /// Preferred alignment if NeedsPreferredAlignment is true, otherwise is the
+  /// ABI alignment.
+  unsigned getTypeAlignIfKnown(QualType T,
+   bool NeedsPreferredAlignment = false) const;
 
   /// Return the ABI-specified alignment of a (complete) type \p T, in
   /// characters.
   CharUnits getTypeAlignInChars(QualType T) const;
   CharUnits getTypeAlignInChars(const Type *T) const;
 
+  /// Return the PreferredAlignment of a (complete) type \p T, in
+  /// characters.
+  CharUnits getPreferredTypeAlignInChars(QualType T) const {
+return toCharUnitsFromBits(getPreferredTypeAlign(T));
+  }
+
   /// getTypeUnadjustedAlignInChars - Return the ABI-specified alignment of a 
type,
   /// in characters, before alignment adjustments. This method does not work on
   /// incomplete types.
@@ -2166,7 +2175,12 @@ class ASTContext : public RefCountedBase {
   /// the current target, in bits.
   ///
   /// This can be 
diff erent than the ABI alignment in cases where it is
-  /// beneficial for performance to overalign a data type.
+  /// beneficial for performance or backwards compatibility preserving to
+  /// overalign a data type. (Note: despite the name, the preferred alignment
+  /// is ABI-impacting, and not an optimization.)
+  unsigned getPreferredTypeAlign(QualType T) const {
+return getPreferredTypeAlign(T.getTypePtr());
+  }
   unsigned getPreferredTypeAlign(const Type *T) const;
 
   /// Return the default alignment for __attribute__((aligned)) on

diff  --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index fc7abeaae9b1..376a0b044010 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -1836,7 +1836,8 @@ bool ASTContext::isAlignmentRequired(QualType T) const {
   return isAlignmentRequired(T.getTypePtr());
 }
 
-unsigned ASTContext::getTypeAlignIfKnown(QualType T) const {
+unsigned ASTContext::getTypeAlignIfKnown(QualType T,
+ bool NeedsPreferredAlignment) const {
   // An alignment on a typedef overrides anything else.
   if (const auto *TT = T->getAs())
 if (unsigned Align = TT->getDecl()->getMaxAlignment())
@@ -1845,7 +1846,7 @@ unsigned ASTContext::getTypeAlignIfKnown(QualType T) 
const {
   // If we have an (array of) complete type, we're done.
   T = getBaseElementType(T);
   if (!T->isIncompleteType())
-return getTypeAlign(T);
+return NeedsPreferredAlignment ? getPreferredTypeAlign(T) : 
getTypeAlign(T);
 
   // If we had an array type, its element type might be a typedef
   // type with an alignment attribute.
@@ -2402,7 +2403,8 @@ CharUnits ASTContext::getTypeUnadjustedAlignInChars(const 
Type *T) const {
 /// getPreferredTypeAlign - Return the "preferred" alignment of the specified
 /// type for the current target in bits.  This can be 
diff erent than the ABI
 /// alignment in 

[PATCH] D88536: [clangd] Split DecisionForest Evaluate() into one func per tree.

[Utkarsh Saxena via Phabricator via cfe-commits]

usaxena95 updated this revision to Diff 295284.
usaxena95 added a comment.

Minor formatting fixes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88536

Files:
  clang-tools-extra/clangd/quality/CompletionModelCodegen.py

Index: clang-tools-extra/clangd/quality/CompletionModelCodegen.py
===
--- clang-tools-extra/clangd/quality/CompletionModelCodegen.py
+++ clang-tools-extra/clangd/quality/CompletionModelCodegen.py
@@ -1,7 +1,7 @@
 """Code generator for Code Completion Model Inference.
 
 Tool runs on the Decision Forest model defined in {model} directory.
-It generates two files: {output_dir}/{filename}.h and {output_dir}/{filename}.cpp 
+It generates two files: {output_dir}/{filename}.h and {output_dir}/{filename}.cpp
 The generated files defines the Example class named {cpp_class} having all the features as class members.
 The generated runtime provides an `Evaluate` function which can be used to score a code completion candidate.
 """
@@ -39,34 +39,32 @@
 
 
 def boost_node(n, label, next_label):
-"""Returns code snippet for a leaf/boost node.
-Adds value of leaf to the score and jumps to the root of the next tree."""
-return "%s: Score += %s; goto %s;" % (
-label, n['score'], next_label)
+"""Returns code snippet for a leaf/boost node."""
+return "%s: return %s;" % (label, n['score'])
 
 
 def if_greater_node(n, label, next_label):
 """Returns code snippet for a if_greater node.
-Jumps to true_label if the Example feature (NUMBER) is greater than the threshold. 
-Comparing integers is much faster than comparing floats. Assuming floating points 
+Jumps to true_label if the Example feature (NUMBER) is greater than the threshold.
+Comparing integers is much faster than comparing floats. Assuming floating points
 are represented as IEEE 754, it order-encodes the floats to integers before comparing them.
 Control falls through if condition is evaluated to false."""
 threshold = n["threshold"]
-return "%s: if (E.%s >= %s /*%s*/) goto %s;" % (
-label, n['feature'], order_encode(threshold), threshold, next_label)
+return "%s: if (%s >= %s /*%s*/) goto %s;" % (
+label, n['feature'], order_encode(threshold), threshold, next_label)
 
 
 def if_member_node(n, label, next_label):
 """Returns code snippet for a if_member node.
-Jumps to true_label if the Example feature (ENUM) is present in the set of enum values 
+Jumps to true_label if the Example feature (ENUM) is present in the set of enum values
 described in the node.
 Control falls through if condition is evaluated to false."""
 members = '|'.join([
 "BIT(%s_type::%s)" % (n['feature'], member)
 for member in n["set"]
 ])
-return "%s: if (E.%s & (%s)) goto %s;" % (
-label, n['feature'], members, next_label)
+return "%s: if (%s & (%s)) goto %s;" % (
+label, n['feature'], members, next_label)
 
 
 def node(n, label, next_label):
@@ -94,8 +92,6 @@
 """
 label = "t%d_n%d" % (tree_num, node_num)
 code = []
-if node_num == 0:
-code.append("t%d:" % tree_num)
 
 if t["operation"] == "boost":
 code.append(node(t, label=label, next_label="t%d" % (tree_num + 1)))
@@ -115,11 +111,11 @@
 return code+false_code+true_code, 1+false_size+true_size
 
 
-def gen_header_code(features_json, cpp_class, filename):
+def gen_header_code(features_json, cpp_class, filename, num_trees):
 """Returns code for header declaring the inference runtime.
 
 Declares the Example class named {cpp_class} inside relevant namespaces.
-The Example class contains all the features as class members. This 
+The Example class contains all the features as class members. This
 class can be used to represent a code completion candidate.
 Provides `float Evaluate()` function which can be used to score the Example.
 """
@@ -145,7 +141,6 @@
 return """#ifndef %s
 #define %s
 #include 
-#include "llvm/Support/Compiler.h"
 
 %s
 class %s {
@@ -158,18 +153,21 @@
   // Produces an integer that sorts in the same order as F.
   // That is: a < b <==> orderEncode(a) < orderEncode(b).
   static uint32_t OrderEncode(float F);
+
+  // Evaluation functions for each tree.
+%s
+
   friend float Evaluate(const %s&);
 };
 
-// The function may have large number of lines of code. MSAN
-// build times out in such case.
-LLVM_NO_SANITIZE("memory")
 float Evaluate(const %s&);
 %s
 #endif // %s
 """ % (guard, guard, cpp_class.ns_begin(), cpp_class.name, nline.join(setters),
-nline.join(class_members), cpp_class.name, cpp_class.name,
-cpp_class.ns_end(), guard)
+nline.join(class_members),
+nline.join(["  float EvaluateTree%d() const;" % tree_num
+for tree_num in range(num_trees)]),
+

[PATCH] D86790: [FE] Use preferred alignment instead of ABI alignment for complete object when applicable

[Xiangling Liao via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG3a7487f903e2: [FE] Use preferred alignment instead of ABI 
alignment for complete object when… (authored by Xiangling_L).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86790

Files:
  clang/include/clang/AST/ASTContext.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/CodeGen/CGExprCXX.cpp
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/lib/CodeGen/TargetInfo.cpp
  clang/test/CodeGen/aix-alignment.c
  clang/test/CodeGenCXX/aix-alignment.cpp

Index: clang/test/CodeGenCXX/aix-alignment.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/aix-alignment.cpp
@@ -0,0 +1,40 @@
+// REQUIRES: powerpc-registered-target
+// RUN: %clang_cc1 -triple powerpc-unknown-aix \
+// RUN: -emit-llvm -o - -x c++ %s | \
+// RUN:   FileCheck %s --check-prefixes=AIX,AIX32
+// RUN: %clang_cc1 -triple powerpc64-unknown-aix \
+// RUN: -emit-llvm -o - %s -x c++| \
+// RUN:   FileCheck %s --check-prefixes=AIX,AIX64
+
+struct B {
+  double d;
+  ~B() {}
+};
+
+// AIX32: %call = call noalias nonnull i8* @_Znam(i32 8)
+// AIX64: %call = call noalias nonnull i8* @_Znam(i64 8)
+B *allocBp() { return new B[0]; }
+
+// AIX-LABEL: delete.notnull:
+// AIX32: %0 = bitcast %struct.B* %call to i8*
+// AIX32: %1 = getelementptr inbounds i8, i8* %0, i32 -8
+// AIX32: %2 = getelementptr inbounds i8, i8* %1, i32 4
+// AIX32: %3 = bitcast i8* %2 to i32*
+// AIX64: %0 = bitcast %struct.B* %call to i8*
+// AIX64: %1 = getelementptr inbounds i8, i8* %0, i64 -8
+// AIX64: %2 = bitcast i8* %1 to i64*
+void bar() { delete[] allocBp(); }
+
+typedef struct D {
+  double d;
+  int i;
+
+  ~D(){};
+} D;
+
+// AIX: define void @_Z3foo1D(%struct.D* noalias sret align 4 %agg.result, %struct.D* %x)
+// AIX:   %1 = bitcast %struct.D* %agg.result to i8*
+// AIX:   %2 = bitcast %struct.D* %x to i8*
+// AIX32  call void @llvm.memcpy.p0i8.p0i8.i32(i8* align 4 %1, i8* align 4 %2, i32 16, i1 false)
+// AIX64: call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 4 %1, i8* align 4 %2, i64 16, i1 false)
+D foo(D x) { return x; }
Index: clang/test/CodeGen/aix-alignment.c
===
--- /dev/null
+++ clang/test/CodeGen/aix-alignment.c
@@ -0,0 +1,41 @@
+// REQUIRES: powerpc-registered-target
+// RUN: %clang_cc1 -triple powerpc-unknown-aix -emit-llvm -o - %s | \
+// RUN:   FileCheck %s --check-prefixes=AIX,AIX32
+// RUN: %clang_cc1 -triple powerpc64-unknown-aix -emit-llvm -o - %s | \
+// RUN:   FileCheck %s --check-prefixes=AIX,AIX64
+
+// AIX: @d = global double 0.00e+00, align 8
+double d;
+
+typedef struct {
+  double d;
+  int i;
+} StructDouble;
+
+// AIX: @d1 = global %struct.StructDouble zeroinitializer, align 8
+StructDouble d1;
+
+// AIX: double @retDouble(double %x)
+// AIX: %x.addr = alloca double, align 8
+// AIX: store double %x, double* %x.addr, align 8
+// AIX: load double, double* %x.addr, align 8
+// AIX: ret double %0
+double retDouble(double x) { return x; }
+
+// AIX32: define void @bar(%struct.StructDouble* noalias sret align 4 %agg.result, %struct.StructDouble* byval(%struct.StructDouble) align 4 %x)
+// AIX64: define void @bar(%struct.StructDouble* noalias sret align 4 %agg.result, %struct.StructDouble* byval(%struct.StructDouble) align 8 %x)
+// AIX: %0 = bitcast %struct.StructDouble* %agg.result to i8*
+// AIX: %1 = bitcast %struct.StructDouble* %x to i8*
+// AIX32:   call void @llvm.memcpy.p0i8.p0i8.i32(i8* align 4 %0, i8* align 4 %1, i32 16, i1 false)
+// AIX64:   call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 4 %0, i8* align 8 %1, i64 16, i1 false)
+StructDouble bar(StructDouble x) { return x; }
+
+// AIX:   define void @foo(double* %out, double* %in)
+// AIX32:   %0 = load double*, double** %in.addr, align 4
+// AIX64:   %0 = load double*, double** %in.addr, align 8
+// AIX: %1 = load double, double* %0, align 4
+// AIX: %mul = fmul double %1, 2.00e+00
+// AIX32:   %2 = load double*, double** %out.addr, align 4
+// AIX64:   %2 = load double*, double** %out.addr, align 8
+// AIX: store double %mul, double* %2, align 4
+void foo(double *out, double *in) { *out = *in * 2; }
Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -4512,8 +4512,6 @@
   if (RetTy->isVoidType())
 return ABIArgInfo::getIgnore();
 
-  // TODO:  Evaluate if AIX power alignment rule would have an impact on the
-  // alignment here.
   if (isAggregateTypeForABI(RetTy))
 return getNaturalAlignIndirect(RetTy);
 
@@ -4530,8 +4528,6 @@
   if (Ty->isVectorType())
 llvm::report_fatal_error("vector type is not supported on AIX yet");
 
-  // TODO:  Evaluate if AIX power alignment rule would have an impact on the
-  // alignment here.
   

[PATCH] D88260: [NFC][FE] Replace TypeSize with StorageUnitSize

[Xiangling Liao via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG944691f0b7fa: [NFC][FE] Replace TypeSize with 
StorageUnitSize (authored by Xiangling_L).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88260

Files:
  clang/lib/AST/RecordLayoutBuilder.cpp

Index: clang/lib/AST/RecordLayoutBuilder.cpp
===
--- clang/lib/AST/RecordLayoutBuilder.cpp
+++ clang/lib/AST/RecordLayoutBuilder.cpp
@@ -622,9 +622,10 @@
   /// an adjacent bitfield if necessary.  The unit in question is usually
   /// a byte, but larger units are used if IsMsStruct.
   unsigned char UnfilledBitsInLastUnit;
-  /// LastBitfieldTypeSize - If IsMsStruct, represents the size of the type
-  /// of the previous field if it was a bitfield.
-  unsigned char LastBitfieldTypeSize;
+
+  /// LastBitfieldStorageUnitSize - If IsMsStruct, represents the size of the
+  /// storage unit of the previous field if it was a bitfield.
+  unsigned char LastBitfieldStorageUnitSize;
 
   /// MaxFieldAlignment - The maximum allowed field alignment. This is set by
   /// #pragma pack.
@@ -693,7 +694,7 @@
 UnadjustedAlignment(CharUnits::One()), UseExternalLayout(false),
 InferAlignment(false), Packed(false), IsUnion(false),
 IsMac68kAlign(false), IsMsStruct(false), UnfilledBitsInLastUnit(0),
-LastBitfieldTypeSize(0), MaxFieldAlignment(CharUnits::Zero()),
+LastBitfieldStorageUnitSize(0), MaxFieldAlignment(CharUnits::Zero()),
 DataSize(0), NonVirtualSize(CharUnits::Zero()),
 NonVirtualAlignment(CharUnits::One()),
 PreferredNVAlignment(CharUnits::One()),
@@ -708,7 +709,7 @@
 
   void LayoutFields(const RecordDecl *D);
   void LayoutField(const FieldDecl *D, bool InsertExtraPadding);
-  void LayoutWideBitField(uint64_t FieldSize, uint64_t TypeSize,
+  void LayoutWideBitField(uint64_t FieldSize, uint64_t StorageUnitSize,
   bool FieldPacked, const FieldDecl *D);
   void LayoutBitField(const FieldDecl *D);
 
@@ -1451,7 +1452,7 @@
 }
 
 void ItaniumRecordLayoutBuilder::LayoutWideBitField(uint64_t FieldSize,
-uint64_t TypeSize,
+uint64_t StorageUnitSize,
 bool FieldPacked,
 const FieldDecl *D) {
   assert(Context.getLangOpts().CPlusPlus &&
@@ -1481,7 +1482,7 @@
 
   // We're not going to use any of the unfilled bits in the last byte.
   UnfilledBitsInLastUnit = 0;
-  LastBitfieldTypeSize = 0;
+  LastBitfieldStorageUnitSize = 0;
 
   uint64_t FieldOffset;
   uint64_t UnpaddedFieldOffset = getDataSizeInBits() - UnfilledBitsInLastUnit;
@@ -1520,7 +1521,7 @@
   bool FieldPacked = Packed || D->hasAttr();
   uint64_t FieldSize = D->getBitWidthValue(Context);
   TypeInfo FieldInfo = Context.getTypeInfo(D->getType());
-  uint64_t TypeSize = FieldInfo.Width;
+  uint64_t StorageUnitSize = FieldInfo.Width;
   unsigned FieldAlign = FieldInfo.Align;
 
   // UnfilledBitsInLastUnit is the difference between the end of the
@@ -1529,7 +1530,7 @@
   // first bit offset available for non-bitfields).  The current data
   // size in bits is always a multiple of the char size; additionally,
   // for ms_struct records it's also a multiple of the
-  // LastBitfieldTypeSize (if set).
+  // LastBitfieldStorageUnitSize (if set).
 
   // The struct-layout algorithm is dictated by the platform ABI,
   // which in principle could use almost any rules it likes.  In
@@ -1583,26 +1584,26 @@
   // First, some simple bookkeeping to perform for ms_struct structs.
   if (IsMsStruct) {
 // The field alignment for integer types is always the size.
-FieldAlign = TypeSize;
+FieldAlign = StorageUnitSize;
 
 // If the previous field was not a bitfield, or was a bitfield
 // with a different storage unit size, or if this field doesn't fit into
 // the current storage unit, we're done with that storage unit.
-if (LastBitfieldTypeSize != TypeSize ||
+if (LastBitfieldStorageUnitSize != StorageUnitSize ||
 UnfilledBitsInLastUnit < FieldSize) {
   // Also, ignore zero-length bitfields after non-bitfields.
-  if (!LastBitfieldTypeSize && !FieldSize)
+  if (!LastBitfieldStorageUnitSize && !FieldSize)
 FieldAlign = 1;
 
   UnfilledBitsInLastUnit = 0;
-  LastBitfieldTypeSize = 0;
+  LastBitfieldStorageUnitSize = 0;
 }
   }
 
   // If the field is wider than its declared type, it follows
   // different rules in all cases.
-  if (FieldSize > TypeSize) {
-LayoutWideBitField(FieldSize, TypeSize, FieldPacked, D);
+  if (FieldSize > StorageUnitSize) {
+LayoutWideBitField(FieldSize, StorageUnitSize, FieldPacked, D);
 return;
   }
 
@@ -1686,7 +1687,7 @@
 // Compute the real 

[clang] 944691f - [NFC][FE] Replace TypeSize with StorageUnitSize

[Xiangling Liao via cfe-commits]

Author: Xiangling Liao
Date: 2020-09-30T10:32:53-04:00
New Revision: 944691f0b7fa8d99790a4544545e55f014c37295

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

LOG: [NFC][FE] Replace TypeSize with StorageUnitSize

On some targets like AIX, last bitfield size is not always equal to last
bitfield type size. Some bitfield like bool will have the same alignment
as [unsigned]. So we'd like to use a more general term `StorageUnit` to
replace type in this field.

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

Added: 


Modified: 
clang/lib/AST/RecordLayoutBuilder.cpp

Removed: 




diff  --git a/clang/lib/AST/RecordLayoutBuilder.cpp 
b/clang/lib/AST/RecordLayoutBuilder.cpp
index 715b629e290d..1c185bb08212 100644
--- a/clang/lib/AST/RecordLayoutBuilder.cpp
+++ b/clang/lib/AST/RecordLayoutBuilder.cpp
@@ -622,9 +622,10 @@ class ItaniumRecordLayoutBuilder {
   /// an adjacent bitfield if necessary.  The unit in question is usually
   /// a byte, but larger units are used if IsMsStruct.
   unsigned char UnfilledBitsInLastUnit;
-  /// LastBitfieldTypeSize - If IsMsStruct, represents the size of the type
-  /// of the previous field if it was a bitfield.
-  unsigned char LastBitfieldTypeSize;
+
+  /// LastBitfieldStorageUnitSize - If IsMsStruct, represents the size of the
+  /// storage unit of the previous field if it was a bitfield.
+  unsigned char LastBitfieldStorageUnitSize;
 
   /// MaxFieldAlignment - The maximum allowed field alignment. This is set by
   /// #pragma pack.
@@ -693,7 +694,7 @@ class ItaniumRecordLayoutBuilder {
 UnadjustedAlignment(CharUnits::One()), UseExternalLayout(false),
 InferAlignment(false), Packed(false), IsUnion(false),
 IsMac68kAlign(false), IsMsStruct(false), UnfilledBitsInLastUnit(0),
-LastBitfieldTypeSize(0), MaxFieldAlignment(CharUnits::Zero()),
+LastBitfieldStorageUnitSize(0), MaxFieldAlignment(CharUnits::Zero()),
 DataSize(0), NonVirtualSize(CharUnits::Zero()),
 NonVirtualAlignment(CharUnits::One()),
 PreferredNVAlignment(CharUnits::One()),
@@ -708,7 +709,7 @@ class ItaniumRecordLayoutBuilder {
 
   void LayoutFields(const RecordDecl *D);
   void LayoutField(const FieldDecl *D, bool InsertExtraPadding);
-  void LayoutWideBitField(uint64_t FieldSize, uint64_t TypeSize,
+  void LayoutWideBitField(uint64_t FieldSize, uint64_t StorageUnitSize,
   bool FieldPacked, const FieldDecl *D);
   void LayoutBitField(const FieldDecl *D);
 
@@ -1451,7 +1452,7 @@ roundUpSizeToCharAlignment(uint64_t Size,
 }
 
 void ItaniumRecordLayoutBuilder::LayoutWideBitField(uint64_t FieldSize,
-uint64_t TypeSize,
+uint64_t StorageUnitSize,
 bool FieldPacked,
 const FieldDecl *D) {
   assert(Context.getLangOpts().CPlusPlus &&
@@ -1481,7 +1482,7 @@ void 
ItaniumRecordLayoutBuilder::LayoutWideBitField(uint64_t FieldSize,
 
   // We're not going to use any of the unfilled bits in the last byte.
   UnfilledBitsInLastUnit = 0;
-  LastBitfieldTypeSize = 0;
+  LastBitfieldStorageUnitSize = 0;
 
   uint64_t FieldOffset;
   uint64_t UnpaddedFieldOffset = getDataSizeInBits() - UnfilledBitsInLastUnit;
@@ -1520,7 +1521,7 @@ void ItaniumRecordLayoutBuilder::LayoutBitField(const 
FieldDecl *D) {
   bool FieldPacked = Packed || D->hasAttr();
   uint64_t FieldSize = D->getBitWidthValue(Context);
   TypeInfo FieldInfo = Context.getTypeInfo(D->getType());
-  uint64_t TypeSize = FieldInfo.Width;
+  uint64_t StorageUnitSize = FieldInfo.Width;
   unsigned FieldAlign = FieldInfo.Align;
 
   // UnfilledBitsInLastUnit is the 
diff erence between the end of the
@@ -1529,7 +1530,7 @@ void ItaniumRecordLayoutBuilder::LayoutBitField(const 
FieldDecl *D) {
   // first bit offset available for non-bitfields).  The current data
   // size in bits is always a multiple of the char size; additionally,
   // for ms_struct records it's also a multiple of the
-  // LastBitfieldTypeSize (if set).
+  // LastBitfieldStorageUnitSize (if set).
 
   // The struct-layout algorithm is dictated by the platform ABI,
   // which in principle could use almost any rules it likes.  In
@@ -1583,26 +1584,26 @@ void ItaniumRecordLayoutBuilder::LayoutBitField(const 
FieldDecl *D) {
   // First, some simple bookkeeping to perform for ms_struct structs.
   if (IsMsStruct) {
 // The field alignment for integer types is always the size.
-FieldAlign = TypeSize;
+FieldAlign = StorageUnitSize;
 
 // If the previous field was not a bitfield, or was a bitfield
 // with a 
diff erent storage unit size, or if this field 

[PATCH] D88477: [analyzer] Overwrite cast type in getBinding only if that was null originally

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

steakhal added a comment.

In D88477#2301641 , @NoQ wrote:

> A load from a region of type `T` should always yield a value of type `T` 
> because that's what a load is.
>
> I'd rather have it as an assertion: if the region is typed, the type 
> shouldn't be specified. If such assertion is hard to satisfy, we could allow 
> the same canonical type to be specified. But having a conflict between two 
> sources of type information and resolving it by arbitrarily choosing one of 
> them sounds scary and unreliable. This patch doesn't eliminate the source of 
> the conflict, it simply changes the way we flip the coin to resolve it.

Oh, now I see. Thank you.

> Normally then loading through a casted pointer the conflict is resolved by 
> representing the casted pointer differently, so that the pointer appeared to 
> be a `TypedValueRegion` of the correct type. Namely, it is achieved by 
> wrapping it into an `ElementRegion` with index 0 and the correct type.

Yes, I have seen that many times, now it's clear why :)

> Why didn't the same occur in this case? Do I understand correctly that `L` is 
> just ``, not `{b, 0 S32b, unsigned char **}`?

In the Summary's example, at location `#2` the value of `**p` is 
`{SymRegion{reg_$0},0 S64b,unsigned char}` - which is exactly 
what we stored at line `#1`.

  void test(int *a, char ***b, float *c) {
*(unsigned char **)b = (unsigned char *)a; // #1
if (**b == 0) // no-crash, #2
  ;
// ...
  }

Dereferencing this location `**b` would result in a typed region of type 
`unsigned char`.
I suspect that the analyzer thinks that following a type-punned pointer will 
point to an object of the static type - which seems reasonable to me.

However, breaks the //invariant// that you said. Since we have mismatching 
types, namely `unsigned char` and `char*` - the type of the `TypedValueRegion` 
and the explicitly specified `LoadTy` respectively.

> [...] if the region is typed, the type shouldn't be specified.

IMO, we should relax this invariant in the following way:
If the region is typed and the type also explicitly specified, we will perform 
a cast to the explicitly stated type.
In other words, overwrite the `LoadTy` only if that was not given.

---

Another possibility could be to change the type of the region at the //bind//. 
By doing that, the Region's type would match the static type so any load 
afterward would use the correct //static// type as it should and we could 
assert your //relaxed invariant// requiring matching types when both are 
specified.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88477

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

[PATCH] D88567: [clangd] Fix invalid UTF8 when extracting doc comments.

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG216af81c39d1: [clangd] Fix invalid UTF8 when extracting doc 
comments. (authored by sammccall).

Changed prior to commit:
  https://reviews.llvm.org/D88567?vs=295265=295266#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88567

Files:
  clang-tools-extra/clangd/CodeCompletionStrings.cpp
  clang-tools-extra/clangd/unittests/CodeCompletionStringsTests.cpp
  clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp


Index: clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
===
--- clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
+++ clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
@@ -1606,11 +1606,11 @@
   // Extracted from boost/spirit/home/support/char_encoding/iso8859_1.hpp
   // This looks like UTF-8 and fools clang, but has high-ISO-8859-1 comments.
   const char *Header = "int PUNCT = 0;\n"
-   "int types[] = { /* \xa1 */PUNCT };";
+   "/* \xa1 */ int types[] = { /* \xa1 */PUNCT };";
   CollectorOpts.RefFilter = RefKind::All;
   CollectorOpts.RefsInHeaders = true;
   runSymbolCollector(Header, "");
-  EXPECT_THAT(Symbols, Contains(QName("types")));
+  EXPECT_THAT(Symbols, Contains(AllOf(QName("types"), Doc("\xef\xbf\xbd ";
   EXPECT_THAT(Symbols, Contains(QName("PUNCT")));
   // Reference is stored, although offset within line is not reliable.
   EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "PUNCT").ID, _)));
Index: clang-tools-extra/clangd/unittests/CodeCompletionStringsTests.cpp
===
--- clang-tools-extra/clangd/unittests/CodeCompletionStringsTests.cpp
+++ clang-tools-extra/clangd/unittests/CodeCompletionStringsTests.cpp
@@ -7,6 +7,7 @@
 
//===--===//
 
 #include "CodeCompletionStrings.h"
+#include "TestTU.h"
 #include "clang/Sema/CodeCompleteConsumer.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
@@ -56,6 +57,14 @@
 "Annotation: Ano\n\nIs this brief?");
 }
 
+TEST_F(CompletionStringTest, GetDeclCommentBadUTF8) {
+  //  is not a valid byte here, should be replaced by encoded .
+  auto TU = TestTU::withCode("/*x\xffy*/ struct X;");
+  auto AST = TU.build();
+  EXPECT_EQ("x\xef\xbf\xbdy",
+getDeclComment(AST.getASTContext(), findDecl(AST, "X")));
+}
+
 TEST_F(CompletionStringTest, MultipleAnnotations) {
   Builder.AddAnnotation("Ano1");
   Builder.AddAnnotation("Ano2");
Index: clang-tools-extra/clangd/CodeCompletionStrings.cpp
===
--- clang-tools-extra/clangd/CodeCompletionStrings.cpp
+++ clang-tools-extra/clangd/CodeCompletionStrings.cpp
@@ -12,6 +12,7 @@
 #include "clang/AST/RawCommentList.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Sema/CodeCompleteConsumer.h"
+#include "llvm/Support/JSON.h"
 #include 
 #include 
 
@@ -86,7 +87,12 @@
   assert(!Ctx.getSourceManager().isLoadedSourceLocation(RC->getBeginLoc()));
   std::string Doc =
   RC->getFormattedText(Ctx.getSourceManager(), Ctx.getDiagnostics());
-  return looksLikeDocComment(Doc) ? Doc : "";
+  if (!looksLikeDocComment(Doc))
+return "";
+  // Clang requires source to be UTF-8, but doesn't enforce this in comments.
+  if (!llvm::json::isUTF8(Doc))
+Doc = llvm::json::fixUTF8(Doc);
+  return Doc;
 }
 
 void getSignature(const CodeCompletionString , std::string *Signature,


Index: clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
===
--- clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
+++ clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
@@ -1606,11 +1606,11 @@
   // Extracted from boost/spirit/home/support/char_encoding/iso8859_1.hpp
   // This looks like UTF-8 and fools clang, but has high-ISO-8859-1 comments.
   const char *Header = "int PUNCT = 0;\n"
-   "int types[] = { /* \xa1 */PUNCT };";
+   "/* \xa1 */ int types[] = { /* \xa1 */PUNCT };";
   CollectorOpts.RefFilter = RefKind::All;
   CollectorOpts.RefsInHeaders = true;
   runSymbolCollector(Header, "");
-  EXPECT_THAT(Symbols, Contains(QName("types")));
+  EXPECT_THAT(Symbols, Contains(AllOf(QName("types"), Doc("\xef\xbf\xbd ";
   EXPECT_THAT(Symbols, Contains(QName("PUNCT")));
   // Reference is stored, although offset within line is not reliable.
   EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "PUNCT").ID, _)));
Index: clang-tools-extra/clangd/unittests/CodeCompletionStringsTests.cpp
===
--- clang-tools-extra/clangd/unittests/CodeCompletionStringsTests.cpp
+++ 

[clang-tools-extra] 216af81 - [clangd] Fix invalid UTF8 when extracting doc comments.

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2020-09-30T16:05:12+02:00
New Revision: 216af81c39d1cc4e90af7b991d517c4c7acc912e

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

LOG: [clangd] Fix invalid UTF8 when extracting doc comments.

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

Added: 


Modified: 
clang-tools-extra/clangd/CodeCompletionStrings.cpp
clang-tools-extra/clangd/unittests/CodeCompletionStringsTests.cpp
clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/CodeCompletionStrings.cpp 
b/clang-tools-extra/clangd/CodeCompletionStrings.cpp
index ef44c153425a..d4a3bdafcae0 100644
--- a/clang-tools-extra/clangd/CodeCompletionStrings.cpp
+++ b/clang-tools-extra/clangd/CodeCompletionStrings.cpp
@@ -12,6 +12,7 @@
 #include "clang/AST/RawCommentList.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Sema/CodeCompleteConsumer.h"
+#include "llvm/Support/JSON.h"
 #include 
 #include 
 
@@ -86,7 +87,12 @@ std::string getDeclComment(const ASTContext , const 
NamedDecl ) {
   assert(!Ctx.getSourceManager().isLoadedSourceLocation(RC->getBeginLoc()));
   std::string Doc =
   RC->getFormattedText(Ctx.getSourceManager(), Ctx.getDiagnostics());
-  return looksLikeDocComment(Doc) ? Doc : "";
+  if (!looksLikeDocComment(Doc))
+return "";
+  // Clang requires source to be UTF-8, but doesn't enforce this in comments.
+  if (!llvm::json::isUTF8(Doc))
+Doc = llvm::json::fixUTF8(Doc);
+  return Doc;
 }
 
 void getSignature(const CodeCompletionString , std::string *Signature,

diff  --git a/clang-tools-extra/clangd/unittests/CodeCompletionStringsTests.cpp 
b/clang-tools-extra/clangd/unittests/CodeCompletionStringsTests.cpp
index 2531922a5ca1..7aace938b70c 100644
--- a/clang-tools-extra/clangd/unittests/CodeCompletionStringsTests.cpp
+++ b/clang-tools-extra/clangd/unittests/CodeCompletionStringsTests.cpp
@@ -7,6 +7,7 @@
 
//===--===//
 
 #include "CodeCompletionStrings.h"
+#include "TestTU.h"
 #include "clang/Sema/CodeCompleteConsumer.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
@@ -56,6 +57,14 @@ TEST_F(CompletionStringTest, DocumentationWithAnnotation) {
 "Annotation: Ano\n\nIs this brief?");
 }
 
+TEST_F(CompletionStringTest, GetDeclCommentBadUTF8) {
+  //  is not a valid byte here, should be replaced by encoded .
+  auto TU = TestTU::withCode("/*x\xffy*/ struct X;");
+  auto AST = TU.build();
+  EXPECT_EQ("x\xef\xbf\xbdy",
+getDeclComment(AST.getASTContext(), findDecl(AST, "X")));
+}
+
 TEST_F(CompletionStringTest, MultipleAnnotations) {
   Builder.AddAnnotation("Ano1");
   Builder.AddAnnotation("Ano2");

diff  --git a/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp 
b/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
index 3940946d8016..80995baf946f 100644
--- a/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
+++ b/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
@@ -1606,11 +1606,11 @@ TEST_F(SymbolCollectorTest, BadUTF8) {
   // Extracted from boost/spirit/home/support/char_encoding/iso8859_1.hpp
   // This looks like UTF-8 and fools clang, but has high-ISO-8859-1 comments.
   const char *Header = "int PUNCT = 0;\n"
-   "int types[] = { /* \xa1 */PUNCT };";
+   "/* \xa1 */ int types[] = { /* \xa1 */PUNCT };";
   CollectorOpts.RefFilter = RefKind::All;
   CollectorOpts.RefsInHeaders = true;
   runSymbolCollector(Header, "");
-  EXPECT_THAT(Symbols, Contains(QName("types")));
+  EXPECT_THAT(Symbols, Contains(AllOf(QName("types"), Doc("\xef\xbf\xbd ";
   EXPECT_THAT(Symbols, Contains(QName("PUNCT")));
   // Reference is stored, although offset within line is not reliable.
   EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "PUNCT").ID, _)));



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

[PATCH] D88567: [clangd] Fix invalid UTF8 when extracting doc comments.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

In D88567#2303332 , @kadircet wrote:

> thanks, LGTM!
>
> Should we also have another test for SymbolCollector, to ensure we don't 
> regress this somehow in the future?

We had a SymbolCollector test for the boost case so I modified it to add a doc 
comment.




Comment at: clang-tools-extra/clangd/CodeCompletionStrings.cpp:93
+  // Clang requires source to be UTF-8, but doesn't enforce this in comments.
+  if (!llvm::json::isUTF8(Doc))
+Doc = llvm::json::fixUTF8(Doc);

kadircet wrote:
> it is always surprising to have these helpers in json library :D (just 
> talking out loud)
Yeah. They're just wrappers around functions from `ConvertUTF.h`.
Do you want a patch to move them there?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88567

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

[PATCH] D88567: [clangd] Fix invalid UTF8 when extracting doc comments.

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

thanks, LGTM!

Should we also have another test for SymbolCollector, to ensure we don't 
regress this somehow in the future?




Comment at: clang-tools-extra/clangd/CodeCompletionStrings.cpp:93
+  // Clang requires source to be UTF-8, but doesn't enforce this in comments.
+  if (!llvm::json::isUTF8(Doc))
+Doc = llvm::json::fixUTF8(Doc);

it is always surprising to have these helpers in json library :D (just talking 
out loud)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88567

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

[PATCH] D88567: [clangd] Fix invalid UTF8 when extracting doc comments.

[Sam McCall via Phabricator via cfe-commits]

sammccall created this revision.
sammccall added a reviewer: kadircet.
Herald added subscribers: cfe-commits, usaxena95, arphaman.
Herald added a project: clang.
sammccall requested review of this revision.
Herald added subscribers: MaskRay, ilya-biryukov.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D88567

Files:
  clang-tools-extra/clangd/CodeCompletionStrings.cpp
  clang-tools-extra/clangd/unittests/CodeCompletionStringsTests.cpp


Index: clang-tools-extra/clangd/unittests/CodeCompletionStringsTests.cpp
===
--- clang-tools-extra/clangd/unittests/CodeCompletionStringsTests.cpp
+++ clang-tools-extra/clangd/unittests/CodeCompletionStringsTests.cpp
@@ -7,6 +7,7 @@
 
//===--===//
 
 #include "CodeCompletionStrings.h"
+#include "TestTU.h"
 #include "clang/Sema/CodeCompleteConsumer.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
@@ -56,6 +57,14 @@
 "Annotation: Ano\n\nIs this brief?");
 }
 
+TEST_F(CompletionStringTest, GetDeclCommentBadUTF8) {
+  //  is not a valid byte here, should be replaced by encoded .
+  auto TU = TestTU::withCode("/*x\xffy*/ struct X;");
+  auto AST = TU.build();
+  EXPECT_EQ("x\xef\xbf\xbdy",
+getDeclComment(AST.getASTContext(), findDecl(AST, "X")));
+}
+
 TEST_F(CompletionStringTest, MultipleAnnotations) {
   Builder.AddAnnotation("Ano1");
   Builder.AddAnnotation("Ano2");
Index: clang-tools-extra/clangd/CodeCompletionStrings.cpp
===
--- clang-tools-extra/clangd/CodeCompletionStrings.cpp
+++ clang-tools-extra/clangd/CodeCompletionStrings.cpp
@@ -12,6 +12,7 @@
 #include "clang/AST/RawCommentList.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Sema/CodeCompleteConsumer.h"
+#include "llvm/Support/JSON.h"
 #include 
 #include 
 
@@ -86,7 +87,12 @@
   assert(!Ctx.getSourceManager().isLoadedSourceLocation(RC->getBeginLoc()));
   std::string Doc =
   RC->getFormattedText(Ctx.getSourceManager(), Ctx.getDiagnostics());
-  return looksLikeDocComment(Doc) ? Doc : "";
+  if (!looksLikeDocComment(Doc))
+return "";
+  // Clang requires source to be UTF-8, but doesn't enforce this in comments.
+  if (!llvm::json::isUTF8(Doc))
+Doc = llvm::json::fixUTF8(Doc);
+  return Doc;
 }
 
 void getSignature(const CodeCompletionString , std::string *Signature,


Index: clang-tools-extra/clangd/unittests/CodeCompletionStringsTests.cpp
===
--- clang-tools-extra/clangd/unittests/CodeCompletionStringsTests.cpp
+++ clang-tools-extra/clangd/unittests/CodeCompletionStringsTests.cpp
@@ -7,6 +7,7 @@
 //===--===//
 
 #include "CodeCompletionStrings.h"
+#include "TestTU.h"
 #include "clang/Sema/CodeCompleteConsumer.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
@@ -56,6 +57,14 @@
 "Annotation: Ano\n\nIs this brief?");
 }
 
+TEST_F(CompletionStringTest, GetDeclCommentBadUTF8) {
+  //  is not a valid byte here, should be replaced by encoded .
+  auto TU = TestTU::withCode("/*x\xffy*/ struct X;");
+  auto AST = TU.build();
+  EXPECT_EQ("x\xef\xbf\xbdy",
+getDeclComment(AST.getASTContext(), findDecl(AST, "X")));
+}
+
 TEST_F(CompletionStringTest, MultipleAnnotations) {
   Builder.AddAnnotation("Ano1");
   Builder.AddAnnotation("Ano2");
Index: clang-tools-extra/clangd/CodeCompletionStrings.cpp
===
--- clang-tools-extra/clangd/CodeCompletionStrings.cpp
+++ clang-tools-extra/clangd/CodeCompletionStrings.cpp
@@ -12,6 +12,7 @@
 #include "clang/AST/RawCommentList.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Sema/CodeCompleteConsumer.h"
+#include "llvm/Support/JSON.h"
 #include 
 #include 
 
@@ -86,7 +87,12 @@
   assert(!Ctx.getSourceManager().isLoadedSourceLocation(RC->getBeginLoc()));
   std::string Doc =
   RC->getFormattedText(Ctx.getSourceManager(), Ctx.getDiagnostics());
-  return looksLikeDocComment(Doc) ? Doc : "";
+  if (!looksLikeDocComment(Doc))
+return "";
+  // Clang requires source to be UTF-8, but doesn't enforce this in comments.
+  if (!llvm::json::isUTF8(Doc))
+Doc = llvm::json::fixUTF8(Doc);
+  return Doc;
 }
 
 void getSignature(const CodeCompletionString , std::string *Signature,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D88275: [ASTMatchers] Add matcher `hasParentIgnoringImplicit`.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel added a comment.

> I'm not concerned about the basic idea behind the proposed matcher, I'm only 
> worried we're making AST matching more confusing by having two different ways 
> of inconsistently accomplishing the same-ish thing.

Aaron, I appreciate this concern, but I would argue that this matcher isn't 
making things any worse. We already have the various `ignoringImplicit` 
matchers, and this new one simply parallels those, but for parents. So, it is 
in some sense "completing" an existing API, which together is an alternative to 
 `traverse`.

We should decide our general policy on these implict matchers vs the traverse 
matchers. Personally, I view `traverse` as an experimental feature that we're 
still figuring out and so would prefer that it not block progress on new 
matchers. But, I'm open to discussion. I can implement this one in my own 
codebase in the meantime if this requires longer discussion (that is, it's not 
blocking me, fwiw).

Also, I don't believe that traverse work in this case. When I change the test 
to use `traverse`, it fails:

  TEST(HasParentIgnoringImplicit, TraverseMatchesExplicitParents) {
std::string Input = R"cc(
  float f() {
  int x = 3;
  int y = 3.0;
  return y;
  }
)cc";
 // ---> Passes because there are not implicit parents.
EXPECT_TRUE(matches(
Input, integerLiteral(traverse(TK_IgnoreImplicitCastsAndParentheses,
   hasParent(varDecl());
  
EXPECT_TRUE(
matches(Input, 
declRefExpr(traverse(TK_IgnoreImplicitCastsAndParentheses,
hasParent(returnStmt());
EXPECT_TRUE(
matches(Input, 
floatLiteral(traverse(TK_IgnoreImplicitCastsAndParentheses,
 hasParent(varDecl());
  }


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88275

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

[PATCH] D88566: be more specific when testing for no fuse-ld warnings

[Ties Stuij via Phabricator via cfe-commits]

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

slight change in commit message


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88566

Files:
  clang/test/Driver/fuse-ld.c


Index: clang/test/Driver/fuse-ld.c
===
--- clang/test/Driver/fuse-ld.c
+++ clang/test/Driver/fuse-ld.c
@@ -12,7 +12,7 @@
 // RUN: %clang %s -### -target x86_64-unknown-linux \
 // RUN:   -fuse-ld=/usr/local/bin/or1k-linux-ld 2>&1 | \
 // RUN:   FileCheck %s --check-prefix=CHECK-NO-WARN
-// CHECK-NO-WARN-NOT: warning:
+// CHECK-NO-WARN-NOT: warning: 'fuse-ld'
 
 // RUN: %clang %s -### \
 // RUN: -target x86_64-unknown-freebsd 2>&1 \


Index: clang/test/Driver/fuse-ld.c
===
--- clang/test/Driver/fuse-ld.c
+++ clang/test/Driver/fuse-ld.c
@@ -12,7 +12,7 @@
 // RUN: %clang %s -### -target x86_64-unknown-linux \
 // RUN:   -fuse-ld=/usr/local/bin/or1k-linux-ld 2>&1 | \
 // RUN:   FileCheck %s --check-prefix=CHECK-NO-WARN
-// CHECK-NO-WARN-NOT: warning:
+// CHECK-NO-WARN-NOT: warning: 'fuse-ld'
 
 // RUN: %clang %s -### \
 // RUN: -target x86_64-unknown-freebsd 2>&1 \
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D88566: be more specific when testing for no fuse-ld warnings

[Ties Stuij via Phabricator via cfe-commits]

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

This test broke for our toolchain as this test triggered unrelated
warnings. Being more specific about not expecting fuse-ld warnings won't
invalidate the test, while playing a bit nicer with possible unrelated features.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D88566

Files:
  clang/test/Driver/fuse-ld.c


Index: clang/test/Driver/fuse-ld.c
===
--- clang/test/Driver/fuse-ld.c
+++ clang/test/Driver/fuse-ld.c
@@ -12,7 +12,7 @@
 // RUN: %clang %s -### -target x86_64-unknown-linux \
 // RUN:   -fuse-ld=/usr/local/bin/or1k-linux-ld 2>&1 | \
 // RUN:   FileCheck %s --check-prefix=CHECK-NO-WARN
-// CHECK-NO-WARN-NOT: warning:
+// CHECK-NO-WARN-NOT: warning: 'fuse-ld'
 
 // RUN: %clang %s -### \
 // RUN: -target x86_64-unknown-freebsd 2>&1 \


Index: clang/test/Driver/fuse-ld.c
===
--- clang/test/Driver/fuse-ld.c
+++ clang/test/Driver/fuse-ld.c
@@ -12,7 +12,7 @@
 // RUN: %clang %s -### -target x86_64-unknown-linux \
 // RUN:   -fuse-ld=/usr/local/bin/or1k-linux-ld 2>&1 | \
 // RUN:   FileCheck %s --check-prefix=CHECK-NO-WARN
-// CHECK-NO-WARN-NOT: warning:
+// CHECK-NO-WARN-NOT: warning: 'fuse-ld'
 
 // RUN: %clang %s -### \
 // RUN: -target x86_64-unknown-freebsd 2>&1 \
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D88370: Emit predefined macro for wavefront size for amdgcn

[Matt Arsenault via Phabricator via cfe-commits]

arsenm added inline comments.



Comment at: clang/test/Driver/amdgpu-macros.cl:380-381
+// RUN:   -mno-wavefrontsize64 %s 2>&1 | FileCheck --check-prefixes=WAVE64 %s
+// RUN: %clang -E -dM -target amdgcn -mcpu=gfx1010 -mwavefrontsize64 \
+// RUN:   -mno-wavefrontsize64 %s 2>&1 | FileCheck --check-prefixes=WAVE32 %s
+// WAVE64-DAG: #define __AMDGCN_WAVEFRONT_SIZE 64

Can you also check with the flags reversed, -mno-wavefrontsize64 
-mwavefrontsize64 


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

https://reviews.llvm.org/D88370

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

[PATCH] D88526: [clang][Sema] Fix PR47676: Handle dependent AltiVec C-style cast

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88526

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

[PATCH] D88363: [CodeGen] Improve likelihood attribute branch weights

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:1622
 EmitBranchOnBoolExpr(CondOp->getCond(), LHSBlock, RHSBlock,
- getProfileCount(CondOp), Weights);
+ getProfileCount(CondOp), Stmt::LH_None);
 

Why `LH_None` in this case? (Also, given that it's the default value, should 
you skip passing this at all? Or should the parameter not have a default value, 
perhaps?)



Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:1678-1679
 
   // Create branch weights based on the number of times we get here and the
   // number of times the condition should be true.
+  llvm::MDNode *Weights = nullptr;

This comment is now a bit stale.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88363

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

[PATCH] D88019: [analyzer][solver] Fix issue with symbol non-equality tracking

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

In D88019#2296337 , @steakhal wrote:

> In D88019#2291953 , @steakhal wrote:
>
>> What are our options mitigating anything similar happening in the future?
>>
>> This way any change touching the `SymbolicRangeInferrer` and any related 
>> parts of the analyzer seems to be way too fragile.
>> Especially, since we might want to add support for comparing SymSyms, just 
>> like we try to do in D77792 .
>
> What about changing the EXPENSIVE_CHECKS 
> 
>  in the assume function in the following way:
> Convert all range constraints into a Z3 model and check if that is `UNSAT`.
> In that case, we would have returned a state with contradictions, so we would 
> prevent this particular bug from lurking around to bite us later.
>
> And another possibility could be to create a debug checker, which registers 
> to the assume callback and does the same conversion and check.
> This is more appealing to me in some way, like decouples the Z3 dependency 
> from the `ConstraintManager` header.
>
> Which approach should I prefer? @NoQ @vsavchenko @martong @xazax.hun 
> @Szelethus

I like the second approach, i.e. to have a debug checker. But I don't see, how 
would this checker validate all constraints at the moment when they are added 
to the State. And if we don't check all constraints then we might end up 
checking a state that is invalid for a while (i.e. that might became invalid 
previously and not because of the lastly added constraint.) So, essentially, my 
gut feeling is that both approaches should be validating all newly added 
constraints against Z3. And that might be too slow, it would have the same 
speed as using z3 instead of the range based solver.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88019

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