[PATCH] D95342: [clang][cli] Store LangStandard::Kind in LangOptions

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 added inline comments.



Comment at: clang/include/clang/Basic/LangOptions.h:18
 #include "clang/Basic/CommentOptions.h"
+#include "clang/Basic/LangStandard.h"
 #include "clang/Basic/LLVM.h"

dexonsmith wrote:
> Nit: clang-format wants you to move this for better `#include` sorting.
Thanks, committing with this fixed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95342

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

[PATCH] D92715: [Clang][RISCV] Define RISC-V V builtin types

[Hsiangkai Wang via Phabricator via cfe-commits]

HsiangKai updated this revision to Diff 319144.
HsiangKai added a comment.

Address @craig.topper's comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92715

Files:
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/AST/Type.h
  clang/include/clang/AST/TypeProperties.td
  clang/include/clang/Basic/RISCVVTypes.def
  clang/include/clang/Basic/TargetInfo.h
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/include/clang/module.modulemap
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/ExprConstant.cpp
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/MicrosoftMangle.cpp
  clang/lib/AST/NSAPI.cpp
  clang/lib/AST/PrintfFormatString.cpp
  clang/lib/AST/Type.cpp
  clang/lib/AST/TypeLoc.cpp
  clang/lib/Basic/TargetInfo.cpp
  clang/lib/Basic/Targets/RISCV.h
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CodeGenTypes.cpp
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/lib/Index/USRGeneration.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Serialization/ASTCommon.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/test/CodeGen/RISCV/riscv-v-debuginfo.c
  clang/test/Sema/riscv-types.c
  clang/tools/libclang/CIndex.cpp

Index: clang/tools/libclang/CIndex.cpp
===
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -1548,6 +1548,8 @@
 #include "clang/Basic/AArch64SVEACLETypes.def"
 #define PPC_VECTOR_TYPE(Name, Id, Size) case BuiltinType::Id:
 #include "clang/Basic/PPCTypes.def"
+#define RVV_TYPE(Name, Id, SingletonId) case BuiltinType::Id:
+#include "clang/Basic/RISCVVTypes.def"
 #define BUILTIN_TYPE(Id, SingletonId)
 #define SIGNED_TYPE(Id, SingletonId) case BuiltinType::Id:
 #define UNSIGNED_TYPE(Id, SingletonId) case BuiltinType::Id:
Index: clang/test/Sema/riscv-types.c
===
--- /dev/null
+++ clang/test/Sema/riscv-types.c
@@ -0,0 +1,136 @@
+// RUN: %clang_cc1 -triple riscv64 -target-feature +experimental-v -ast-print %s \
+// RUN:| FileCheck %s
+
+void bar(void) {
+  // CHECK: __rvv_int64m1_t x0;
+  __rvv_int64m1_t x0;
+
+  // CHECK: __rvv_float64m1_t x1;
+  __rvv_float64m1_t x1;
+
+  // CHECK: __rvv_int64m2_t x2;
+  __rvv_int64m2_t x2;
+
+  // CHECK: __rvv_float64m2_t x3;
+  __rvv_float64m2_t x3;
+
+  // CHECK: __rvv_int64m4_t x4;
+  __rvv_int64m4_t x4;
+
+  // CHECK: __rvv_float64m4_t x5;
+  __rvv_float64m4_t x5;
+
+  // CHECK: __rvv_int64m8_t x6;
+  __rvv_int64m8_t x6;
+
+  // CHECK: __rvv_float64m8_t x7;
+  __rvv_float64m8_t x7;
+
+  // CHECK: __rvv_int32m1_t x8;
+  __rvv_int32m1_t x8;
+
+  // CHECK: __rvv_float32m1_t x9;
+  __rvv_float32m1_t x9;
+
+  // CHECK: __rvv_int32m2_t x10;
+  __rvv_int32m2_t x10;
+
+  // CHECK: __rvv_float32m2_t x11;
+  __rvv_float32m2_t x11;
+
+  // CHECK: __rvv_int32m4_t x12;
+  __rvv_int32m4_t x12;
+
+  // CHECK: __rvv_float32m4_t x13;
+  __rvv_float32m4_t x13;
+
+  // CHECK: __rvv_int32m8_t x14;
+  __rvv_int32m8_t x14;
+
+  // CHECK: __rvv_float32m8_t x15;
+  __rvv_float32m8_t x15;
+
+  // CHECK: __rvv_int16m1_t x16;
+  __rvv_int16m1_t x16;
+
+  // CHECK: __rvv_float16m1_t x17;
+  __rvv_float16m1_t x17;
+
+  // CHECK: __rvv_int16m2_t x18;
+  __rvv_int16m2_t x18;
+
+  // CHECK: __rvv_float16m2_t x19;
+  __rvv_float16m2_t x19;
+
+  // CHECK: __rvv_int16m4_t x20;
+  __rvv_int16m4_t x20;
+
+  // CHECK: __rvv_float16m4_t x21;
+  __rvv_float16m4_t x21;
+
+  // CHECK: __rvv_int16m8_t x22;
+  __rvv_int16m8_t x22;
+
+  // CHECK: __rvv_float16m8_t x23;
+  __rvv_float16m8_t x23;
+
+  // CHECK: __rvv_int8m1_t x24;
+  __rvv_int8m1_t x24;
+
+  // CHECK: __rvv_int8m2_t x25;
+  __rvv_int8m2_t x25;
+
+  // CHECK: __rvv_int8m4_t x26;
+  __rvv_int8m4_t x26;
+
+  // CHECK: __rvv_int8m8_t x27;
+  __rvv_int8m8_t x27;
+
+  // CHECK: __rvv_bool64_t x28;
+  __rvv_bool64_t x28;
+
+  // CHECK: __rvv_bool32_t x29;
+  __rvv_bool32_t x29;
+
+  // CHECK: __rvv_bool16_t x30;
+  __rvv_bool16_t x30;
+
+  // CHECK: __rvv_bool8_t x31;
+  __rvv_bool8_t x31;
+
+  // CHECK: __rvv_bool8_t x32;
+  __rvv_bool8_t x32;
+
+  // CHECK: __rvv_bool8_t x33;
+  __rvv_bool8_t x33;
+
+  // CHECK: __rvv_bool8_t x34;
+  __rvv_bool8_t x34;
+
+  // CHECK: __rvv_int32mf2_t x35;
+  __rvv_int32mf2_t x35;
+
+  // CHECK: __rvv_float32mf2_t x36;
+  __rvv_float32mf2_t x36;
+
+  // CHECK: __rvv_int16mf4_t x37;
+  __rvv_int16mf4_t x37;
+
+  // CHECK: __rvv_float16mf4_t x38;
+  __rvv_float16mf4_t x38;
+
+  // CHECK: __rvv_int16mf2_t x39;
+  __rvv_int16mf2_t x39;
+
+  // CHECK: __rvv_float16mf2_t x40;
+  __rvv_float16mf2_t x40;
+
+  // CHECK: __rvv_int8mf8_t x41;
+  __rvv_int8mf8_t x41;
+
+  // CHECK: __rvv_int8mf4_t x42;
+  __rvv_int8mf4_t x42;
+
+  // CHECK: __rvv_int8mf2_t x43;
+  __rvv_int8mf2_t x43;
+}
Index: clang/test/CodeGen/RISCV/riscv-v-debuginfo.c
===
--- 

[PATCH] D94952: [clangd] Take into account what is in the index (symbols, references, etc.) at indexes merge

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: clang-tools-extra/clangd/index/FileIndex.cpp:433
 PreambleSymbols.update(
-Uri, std::make_unique(std::move(*IF->Symbols)),
+FilePath ? *FilePath : (consumeError(FilePath.takeError()), Uri),
+std::make_unique(std::move(*IF->Symbols)),

ArcsinX wrote:
> sammccall wrote:
> > Is this change related? It changes the key scheme for the preamble index 
> > from URIs to paths, but I'm not sure why.
> > 
> > Do we have multiple URIs pointing to the same path? What are they 
> > concretely?
> This is the main thing in this patch. I will try to explain.
> We use these keys to create the file list, which is used by `indexedFiles()`.
> Currently, the preamble index contains URI's instead of paths (as a file 
> list), that leads to the function returned by `PreambleIndex::indexedFiles()` 
> always return `false` (because we pass to this function paths, not URI's). 
> So, we always take data from the preamble index (but maybe we should not in 
> some cases).
> 
Oooh... I'm not sure how I misunderstood the original so much :-( And I missed 
it in this patch description as well, apologies.

My impression was that the file list was derived from the index data, rather 
than from the keys, which were always intended to be opaque/arbitrary.
(At various times, these have been filenames, URIs, and other things IIRC. And 
until relatively recently, the preamble index keys were the file the preamble 
was built from, not the file containing the symbol!)

It feels like using URIs extracted from symbols might not be *completely* 
robust. Because having CanonicalDeclaration etc set to a file might not line up 
exactly with the idea that we indexed the file. But we do use this partitioning 
for FileShardedIndex, so it has to work well enough.

The advantage of using the extracted URIs would be: also works for 
non-file-sharded indexes like --index-file, avoid a bunch of conversion between 
URI and path, and we get to keep the simpler/flexible design for FileSymbols 
where the key is opaque.

Does this seem feasible to you?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94952

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

[PATCH] D95340: [clang][cli] NFC: Simplify BoolOption API

[Jan Svoboda via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGb61639985e21: [clang][cli] NFC: Simplify BoolOption API 
(authored by jansvoboda11).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95340

Files:
  clang/include/clang/Driver/Options.td

Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -296,61 +296,75 @@
 }
 
 //===--===//
-// BoolOptionBase
+// BoolOption
 //===--===//
 
-// Default value of the keypath associated with a marshalled bool option.
+// The default value of a marshalled key path.
 class Default { code Value = value; }
 
-class FlagPolarity { bit Value = value; }
-def PosFlag : FlagPolarity {}
-def NegFlag : FlagPolarity {}
+// Convenience variables for boolean defaults.
+def DefaultTrue : Default<"true"> {}
+def DefaultFalse : Default<"false"> {}
+
+// The value set to the key path when the flag is present on the command line.
+class Set { bit Value = value; }
+def SetTrue : Set {}
+def SetFalse : Set {}
 
-// Definition of a single command line flag.
-class FlagDef option_flags,
+// Definition of single command line flag. This is an implementation detail, use
+// SetTrueBy or SetFalseBy instead.
+class FlagDef option_flags,
   string help, list implied_by_options = []> {
-  // Negative polarity (false) implies a command line spelling prefixed with
-  // "no-" and a TableGen record whose name is prefixed with "no_".
-  FlagPolarity Polarity = polarity;
+  // The polarity. Besides spelling, this also decides whether the TableGen
+  // record will be prefixed with "no_".
+  bit Polarity = polarity;
 
-  // The value assigned to keypath when the flag is present on the command line.
+  // The value assigned to key path when the flag is present on command line.
   bit Value = value;
 
-  // List of OptionFlag records that control the visibility of the flag in
-  // different scenarios.
+  // OptionFlags that control visibility of the flag in different tools.
   list OptionFlags = option_flags;
 
   // The help text associated with the flag.
   string Help = help;
 
-  // List of options that imply this flag when present on command line.
+  // Options that imply this flag when present on command line.
   list ImpliedBy = implied_by_options;
 }
 
-// Information extending a FlagDef.
-class FlagDefSuffix option_flags, string help> {
+// Additional information to be appended to both positive and negative flag.
+class BothFlags option_flags, string help = ""> {
   list OptionFlags = option_flags;
   string Help = help;
 }
 
-// Extend the flag definition with a suffix.
-class ApplySuffix {
+// Functor that appends the suffix to the base flag definition.
+class ApplySuffix {
   FlagDef Result
 = FlagDef;
+  !listconcat(flag.OptionFlags, suffix.OptionFlags),
+  !strconcat(flag.Help, suffix.Help), flag.ImpliedBy>;
 }
 
-// FlagDef extension. Convenient for creation of TableGen records.
+// Definition of the command line flag with positive spelling, e.g. "-ffoo".
+class PosFlag flags = [], string help = "",
+  list implied_by_options = []>
+  : FlagDef {}
+
+// Definition of the command line flag with negative spelling, e.g. "-fno-foo".
+class NegFlag flags = [], string help = "",
+  list implied_by_options = []>
+  : FlagDef {}
+
+// Expanded FlagDef that's convenient for creation of TableGen records.
 class FlagDefExpanded
   : FlagDef {
   // Name of the TableGen record.
-  string RecordName = prefix#!cond(flag.Polarity.Value : "", true : "no_")#name;
+  string RecordName = prefix#!cond(flag.Polarity : "", true : "no_")#name;
 
   // Spelling of the flag.
-  string Spelling
-= prefix#!cond(flag.Polarity.Value : "", true : "no-")#spelling;
+  string Spelling = prefix#!cond(flag.Polarity : "", true : "no-")#spelling;
 
   // Can the flag be implied by another flag?
   bit CanBeImplied = !not(!empty(flag.ImpliedBy));
@@ -359,9 +373,10 @@
   code ValueAsCode = !cond(flag.Value : "true", true: "false");
 }
 
-// Creates record with a marshalled flag.
-class BoolOptionFlag
+// TableGen record for a single marshalled flag.
+class MarshalledFlagRec
   : Flag<["-"], flag.Spelling>, Flags, HelpText,
 MarshallingInfoBooleanFlag {}
 
 // Generates TableGen records for two command line flags that control the same
-// keypath via the marshalling infrastructure.
+// key path via the marshalling infrastructure.
 // Names of the records consist of the specified prefix, "no_" for the negative
 // flag, and NAME.
-// BoolOption is the API that should be used most of the time. Use this only

[PATCH] D94466: [X86] merge "={eax}" and "~{eax}" into "=" for MSInlineASM

[Freddy, Ye via Phabricator via cfe-commits]

FreddyYe added a comment.

Ping?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94466

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

[PATCH] D91913: Suppress non-conforming GNU paste extension in all standard-conforming modes

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

In D91913#2521031 , @thakis wrote:

> In D91913#2520503 , @hvdijk wrote:
>
>> Chrome `-std=c++*` when building with clang, but `-std=gnu++*` when building 
>> with GCC for exactly this reason, see build/config/compiler/BUILD.gn. Chrome 
>> should use `-std=gnu++*` with clang too.
>
> We (and every other project out there) needs some incremental rollout plan 
> for this. We don't want to allow _more_ GNU features. Please put this behind 
> a `-f` toggle, or revert this for a bit implementing this takes some time.

I'm definitely sympathetic to wanting to avoid further use of GNUisms sneaking 
into your codebase. We're in a tricky position, though, if our choices are that 
either we add a `-f` flag for this, end up maintaining fine-grained GNU 
language mode flags which we don't want, and have a hard time removing those 
flags, or you enable all GNU language changes, and have to work to avoid them 
being used / have a hard time ever switching back to `-std=c++*`. I'd be OK 
with the fine-grained flag if we could reasonably plan to remove after one or 
two Clang releases, but these things have a tendency to never go away.

We have another option: we could restore the old behavior for the Clang 12 
release branch (branching tomorrow!), and add a warning to the release branch 
for uses of the comma paste extension in modes where we want to remove it, so 
that people have a release cycle to transition before we actually make the 
change in anger. That would probably be the most comfortable path forward, with 
the least time pressure all around.

However, there are a couple of additional problems:

- There doesn't seem to be any kind of feature test macro for `__VA_OPT__`, so 
people can't easily use it when available and fall back on comma paste 
otherwise. I've suggested to WG21 that we allow use of `#ifdef __VA_OPT__` (and 
`#ifndef __VA_OPT__` and `defined(__VA_OPT__)`) for this.
- We still retain the comma paste semantics in all cases if there is at least 
one named parameter. Historically, the rationale for that has been that 
omitting the final comma in such cases was ill-formed, but that is no longer 
the case in C++20 onwards, with the advent of `__VA_OPT__`. So we should be 
working towards removing the `getNumParams() < 2` check in at least the C++20 
case.

So perhaps this might be a reasonable plan:

For Clang 12:

- Undo the semantic change and add a warning on comma pastes that will change 
meaning on Clang 13.
- Add feature test support for `__VA_OPT__` via `#ifdef __VA_OPT__`.
- Clearly document in the release notes that this is going to change in Clang 
13, and that `__VA_OPT__` should be used instead for users who don't want to 
enable GNU mode or `-fms-compatibility`.

For Clang 13:

- Disable comma paste for macros with named parameters in `-std=c89` and 
`-std=c++98` through `-std=c++17`; disable comma paste for all macros in 
`-std=c++20` onwards.
- (Also add feature test support for `__VA_OPT__`.)

Thoughts?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91913

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

[PATCH] D95421: [NFC] Refine some uninitialized used variables.

[Pengfei Wang via Phabricator via cfe-commits]

pengfei accepted this revision.
pengfei 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/D95421/new/

https://reviews.llvm.org/D95421

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

[PATCH] D95421: [NFC] Refine some uninitialized used variables.

[Freddy, Ye via Phabricator via cfe-commits]

FreddyYe updated this revision to Diff 319216.
FreddyYe added a comment.

refine according to Lint


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95421

Files:
  clang/lib/CodeGen/CGBlocks.cpp
  llvm/lib/CodeGen/RegisterCoalescer.cpp
  llvm/lib/Target/X86/X86ISelLowering.cpp

Index: llvm/lib/Target/X86/X86ISelLowering.cpp
===
--- llvm/lib/Target/X86/X86ISelLowering.cpp
+++ llvm/lib/Target/X86/X86ISelLowering.cpp
@@ -13830,7 +13830,7 @@
   assert(V2.getSimpleValueType() == MVT::v4f32 && "Bad operand type!");
 
   // Attempt to match the insertps pattern.
-  unsigned InsertPSMask;
+  unsigned InsertPSMask = 0;
   if (!matchShuffleAsInsertPS(V1, V2, InsertPSMask, Zeroable, Mask, DAG))
 return SDValue();
 
Index: llvm/lib/CodeGen/RegisterCoalescer.cpp
===
--- llvm/lib/CodeGen/RegisterCoalescer.cpp
+++ llvm/lib/CodeGen/RegisterCoalescer.cpp
@@ -442,7 +442,7 @@
   Flipped = CrossClass = false;
 
   Register Src, Dst;
-  unsigned SrcSub, DstSub;
+  unsigned SrcSub = 0, DstSub = 0;
   if (!isMoveInstr(TRI, MI, Src, Dst, SrcSub, DstSub))
 return false;
   Partial = SrcSub || DstSub;
@@ -537,7 +537,7 @@
   if (!MI)
 return false;
   Register Src, Dst;
-  unsigned SrcSub, DstSub;
+  unsigned SrcSub = 0, DstSub = 0;
   if (!isMoveInstr(TRI, MI, Src, Dst, SrcSub, DstSub))
 return false;
 
@@ -1590,7 +1590,7 @@
   // CoalescerPair may have a new register class with adjusted subreg indices
   // at this point.
   Register SrcReg, DstReg;
-  unsigned SrcSubIdx, DstSubIdx;
+  unsigned SrcSubIdx = 0, DstSubIdx = 0;
   if(!isMoveInstr(*TRI, CopyMI, SrcReg, DstReg, SrcSubIdx, DstSubIdx))
 return nullptr;
 
@@ -1966,7 +1966,7 @@
 if (!canJoinPhys(CP)) {
   // Before giving up coalescing, if definition of source is defined by
   // trivial computation, try rematerializing it.
-  bool IsDefCopy;
+  bool IsDefCopy = false;
   if (reMaterializeTrivialDef(CP, CopyMI, IsDefCopy))
 return true;
   if (IsDefCopy)
@@ -2005,7 +2005,7 @@
 
 // If definition of source is defined by trivial computation, try
 // rematerializing it.
-bool IsDefCopy;
+bool IsDefCopy = false;
 if (reMaterializeTrivialDef(CP, CopyMI, IsDefCopy))
   return true;
 
@@ -3798,7 +3798,7 @@
   if (!UseTerminalRule)
 return false;
   Register SrcReg, DstReg;
-  unsigned SrcSubReg, DstSubReg;
+  unsigned SrcSubReg = 0, DstSubReg = 0;
   if (!isMoveInstr(*TRI, , SrcReg, DstReg, SrcSubReg, DstSubReg))
 return false;
   // Check if the destination of this copy has any other affinity.
@@ -3823,7 +3823,7 @@
 if ( ==  || !MI.isCopyLike() || MI.getParent() != OrigBB)
   continue;
 Register OtherSrcReg, OtherReg;
-unsigned OtherSrcSubReg, OtherSubReg;
+unsigned OtherSrcSubReg = 0, OtherSubReg = 0;
 if (!isMoveInstr(*TRI, , OtherSrcReg, OtherReg, OtherSrcSubReg,
 OtherSubReg))
   return false;
Index: clang/lib/CodeGen/CGBlocks.cpp
===
--- clang/lib/CodeGen/CGBlocks.cpp
+++ clang/lib/CodeGen/CGBlocks.cpp
@@ -2697,7 +2697,7 @@
   }
 
   bool HasByrefExtendedLayout = false;
-  Qualifiers::ObjCLifetime Lifetime;
+  Qualifiers::ObjCLifetime Lifetime = Qualifiers::OCL_None;
   if (getContext().getByrefLifetime(Ty, Lifetime, HasByrefExtendedLayout) &&
   HasByrefExtendedLayout) {
 /// void *__byref_variable_layout;
@@ -2767,8 +2767,8 @@
   const VarDecl  = *emission.Variable;
   QualType type = D.getType();
 
-  bool HasByrefExtendedLayout;
-  Qualifiers::ObjCLifetime ByrefLifetime;
+  bool HasByrefExtendedLayout = false;
+  Qualifiers::ObjCLifetime ByrefLifetime = Qualifiers::OCL_None;
   bool ByRefHasLifetime =
 getContext().getByrefLifetime(type, ByrefLifetime, HasByrefExtendedLayout);
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D94987: DR39: Perform ambiguous subobject checks for class member access as part of object argument conversion, not as part of name lookup.

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

In D94987#2521774 , @rjmccall wrote:

>> You can. The rule is that you resolve using-declarations to the declarations 
>> they name, and you resolve typedef declarations to the types they name, and 
>> the resulting set of declarations and types must be the same in every 
>> subobject in which there are (non-shadowed) results -- if not, the lookup is 
>> ill-formed. (Put another way, the set of entities found in each subobject 
>> must be the same, but the set of declarations found in each subobject can 
>> differ.)
>
> Okay, makes sense.  So the DeclAccessPairs in the lookup result are basically 
> just the first+best pair we found for any particular entity?

Yes, each pair comprises the declaration from the first subobject, with the 
best unprivileged access for the corresponding declarations in all subobjects.




Comment at: clang/include/clang/AST/CXXInheritance.h:77
 
-  CXXBasePath() = default;
+  /// Additional data stashed on the base path by its consumers.
+  union {

rjmccall wrote:
> rsmith wrote:
> > rjmccall wrote:
> > > Is there a way to know which case we're in, or do different consumers do 
> > > different things?
> > We could use a discriminated union here. Ultimately, I think what we really 
> > want is to provide some opaque storage for the consumer to use, rather than 
> > to have some hard-coded list here. I haven't thought of a good way to 
> > expose that; I don't think it's worth templating the entire base path 
> > finding mechanism to add such storage, and a single pointer isn't large 
> > enough for a `DeclContext::lookup_result`.
> > 
> > Vassil's https://reviews.llvm.org/D91524 will make the `lookup_result` case 
> > fit into a single pointer; once we adopt that, perhaps we can switch to 
> > holding an opaque `void*` here?
> Yeah, that might be cleaner, assuming we really need this.  What are the 
> clients that need to store something specifically in the `CXXBasePath` object 
> and can't just keep it separate?
The main users of this are in name lookup proper (where we store the lookup 
results on the path to avoid redoing the class scope hash table lookup) and now 
in access checking (where we store the most-accessible declaration). I think 
there's a couple of other name-lookup-like clients that also store a lookup 
result here.

In all cases the side storage is just a convenience. But it's probably an 
important convenience; it's awkward for a client to maintain a mapping on the 
side, because the key for that map would end up being the entire base path. 

We could just number the paths as we find them, and let the consumers build a 
`SmallVector` on the side rather than storing data in the `CXXBasePath`s. We'd 
need to be careful about the post-processing pass in `lookupInBases` that 
removes virtual base paths that are shadowed by non-virtual inheritance from 
the vbase, but that seems feasible. Worthwhile?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94987

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

[PATCH] D95017: [clang-format] add case aware include sorting

[Kent Sommer via Phabricator via cfe-commits]

kentsommer added a comment.

In D95017#2522063 , @curdeius wrote:

> Should we then revert ASAP and rework it later? @mydeveloperday

Reverting would also be fine with me. I am happy to drive towards a feature 
(and flags) that everyone is comfortable with in whichever way makes the most 
sense.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95017

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

[PATCH] D95307: [StaticAnalyzer] Add checking for degenerate base class in MemRegion

[Deep Majumder via Phabricator via cfe-commits]

RedDocMD added a comment.

Thanks @NoQ for the tip on the right test command!
My own belief is that this patch is just a hack that squelches the problem, 
instead of solving it. I am limited by my own inexperience with the codebase 
and am trying to learn more to help better.

That said, couple of observations that I made while exploring the bug:

1. Referring to loop in line 1119 of `SimpleSValBuilder.cpp`.  This loop is 
necessary since we need to access the right sub-object. (The test at line 203 
in `pointer-to-member.cpp` is a good demonstration why)
2. However,  when we have static casts on the pointer-to-member, this loop is 
also triggered.

The trouble is that the above two cases behave inconsistently with each other. 
Consider the following example for case 1:

  struct B {
int f;
  };
  struct L1 : public B { };
  struct R1 : public B { };
  struct M : public L1, R1 { };
  struct L2 : public M { };
  struct R2 : public M { };
  struct D2 : public L2, R2 { };
  
  void diamond() {
M m;
  
static_cast()->f = 7;
static_cast()->f = 16;
  
int L1::* pl1 = ::f;
int M::* pm_via_l1 = pl1;
  
int R1::* pr1 = ::f;
int M::* pm_via_r1 = pr1;
  
(m.*(pm_via_l1) == 7); 
(m.*(pm_via_r1) == 16);
  }

This behaves correctly. If we run `I->getType()->dump()` on each iteration of 
the loop, we get something like this:

  RecordType 0x55a70ad60d50 'struct L1'
  `-CXXRecord 0x55a70ad60cb8 'L1'
  RecordType 0x55a70ad60b20 'struct B'
  `-CXXRecord 0x55a70ad60a90 'B'
  2 levels of base specifiers
  RecordType 0x55a70ad60f50 'struct R1'
  `-CXXRecord 0x55a70ad60ec0 'R1'
  RecordType 0x55a70ad60b20 'struct B'
  `-CXXRecord 0x55a70ad60a90 'B'
  2 levels of base specifiers

Running the same analysis for the regressing code:

  struct Base {
int field;
  };
  
  struct Derived : public Base {};
  
  void static_cast_test() {
int Derived::* derived_field = ::field;
Base base;
base.field = 5;
int Base::* base_field = static_cast(derived_field);
(base.*base_field == 5);

we get the following dump;

  RecordType 0x55676ded06a0 'struct Base'
  `-CXXRecord 0x55676ded0610 'Base'
  RecordType 0x55676ded06a0 'struct Base'
  `-CXXRecord 0x55676ded0610 'Base'
  2 base specifier

Clearly, there is a bug elsewhere. The method `PointerToMember::begin()` 
returns an iterator to a list of `CXXBaseSpecifiers`, which according to the 
docs is for keeping track of `static_casts`. However, it seems that the 
base-specifier list is being used for something else. I am not sure in which 
function introduces this problem. Probably somewhere in the ASTVisitor for the 
StaticAnalyzer this problem is introduced. I will keep on digging and add more 
comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95307

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

[PATCH] D95423: [clangd] Add std::size_t to StdSymbol mapping

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

This is a common symbol that's missing from our mapping because
cppreference yields multiple headers.

Add it manually by picking cstddef to prevent insertion of some stdlib-internal
headers instead.

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D95423

Files:
  clang-tools-extra/clangd/index/CanonicalIncludes.cpp


Index: clang-tools-extra/clangd/index/CanonicalIncludes.cpp
===
--- clang-tools-extra/clangd/index/CanonicalIncludes.cpp
+++ clang-tools-extra/clangd/index/CanonicalIncludes.cpp
@@ -92,6 +92,8 @@
 #include "StdSymbolMap.inc"
 // There are two std::move()s, this is by far the most common.
 SYMBOL(move, std::, )
+// There are multiple headers for size_t, pick one.
+SYMBOL(size_t, std::, )
 #undef SYMBOL
 });
 StdSymbolMapping = Symbols;
@@ -99,6 +101,8 @@
 static const auto *CSymbols = new llvm::StringMap({
 #define SYMBOL(Name, NameSpace, Header) {#Name, #Header},
 #include "CSymbolMap.inc"
+// There are multiple headers for size_t, pick one.
+SYMBOL(size_t, None, )
 #undef SYMBOL
 });
 StdSymbolMapping = CSymbols;


Index: clang-tools-extra/clangd/index/CanonicalIncludes.cpp
===
--- clang-tools-extra/clangd/index/CanonicalIncludes.cpp
+++ clang-tools-extra/clangd/index/CanonicalIncludes.cpp
@@ -92,6 +92,8 @@
 #include "StdSymbolMap.inc"
 // There are two std::move()s, this is by far the most common.
 SYMBOL(move, std::, )
+// There are multiple headers for size_t, pick one.
+SYMBOL(size_t, std::, )
 #undef SYMBOL
 });
 StdSymbolMapping = Symbols;
@@ -99,6 +101,8 @@
 static const auto *CSymbols = new llvm::StringMap({
 #define SYMBOL(Name, NameSpace, Header) {#Name, #Header},
 #include "CSymbolMap.inc"
+// There are multiple headers for size_t, pick one.
+SYMBOL(size_t, None, )
 #undef SYMBOL
 });
 StdSymbolMapping = CSymbols;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D95017: [clang-format] add case aware include sorting

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added a comment.

Should we then revert ASAP and rework it later? @mydeveloperday


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95017

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

[PATCH] D95421: [NFC] Refine some uninitialized used variables.

[Freddy, Ye via Phabricator via cfe-commits]

FreddyYe created this revision.
Herald added subscribers: pengfei, tpr, hiraditya, qcolombet, MatzeB.
FreddyYe requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

These warning are reported by static code analysis tool: Klocwork


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D95421

Files:
  clang/lib/CodeGen/CGBlocks.cpp
  llvm/lib/CodeGen/RegisterCoalescer.cpp
  llvm/lib/Target/X86/X86ISelLowering.cpp

Index: llvm/lib/Target/X86/X86ISelLowering.cpp
===
--- llvm/lib/Target/X86/X86ISelLowering.cpp
+++ llvm/lib/Target/X86/X86ISelLowering.cpp
@@ -13830,7 +13830,7 @@
   assert(V2.getSimpleValueType() == MVT::v4f32 && "Bad operand type!");
 
   // Attempt to match the insertps pattern.
-  unsigned InsertPSMask;
+  unsigned InsertPSMask = 0;
   if (!matchShuffleAsInsertPS(V1, V2, InsertPSMask, Zeroable, Mask, DAG))
 return SDValue();
 
Index: llvm/lib/CodeGen/RegisterCoalescer.cpp
===
--- llvm/lib/CodeGen/RegisterCoalescer.cpp
+++ llvm/lib/CodeGen/RegisterCoalescer.cpp
@@ -442,7 +442,7 @@
   Flipped = CrossClass = false;
 
   Register Src, Dst;
-  unsigned SrcSub, DstSub;
+  unsigned SrcSub = 0, DstSub = 0;
   if (!isMoveInstr(TRI, MI, Src, Dst, SrcSub, DstSub))
 return false;
   Partial = SrcSub || DstSub;
@@ -537,7 +537,7 @@
   if (!MI)
 return false;
   Register Src, Dst;
-  unsigned SrcSub, DstSub;
+  unsigned SrcSub = 0, DstSub = 0;
   if (!isMoveInstr(TRI, MI, Src, Dst, SrcSub, DstSub))
 return false;
 
@@ -1590,7 +1590,7 @@
   // CoalescerPair may have a new register class with adjusted subreg indices
   // at this point.
   Register SrcReg, DstReg;
-  unsigned SrcSubIdx, DstSubIdx;
+  unsigned SrcSubIdx = 0, DstSubIdx = 0;
   if(!isMoveInstr(*TRI, CopyMI, SrcReg, DstReg, SrcSubIdx, DstSubIdx))
 return nullptr;
 
@@ -1966,7 +1966,7 @@
 if (!canJoinPhys(CP)) {
   // Before giving up coalescing, if definition of source is defined by
   // trivial computation, try rematerializing it.
-  bool IsDefCopy;
+  bool IsDefCopy = false;
   if (reMaterializeTrivialDef(CP, CopyMI, IsDefCopy))
 return true;
   if (IsDefCopy)
@@ -2005,7 +2005,7 @@
 
 // If definition of source is defined by trivial computation, try
 // rematerializing it.
-bool IsDefCopy;
+bool IsDefCopy = false;
 if (reMaterializeTrivialDef(CP, CopyMI, IsDefCopy))
   return true;
 
@@ -3798,7 +3798,7 @@
   if (!UseTerminalRule)
 return false;
   Register SrcReg, DstReg;
-  unsigned SrcSubReg, DstSubReg;
+  unsigned SrcSubReg = 0, DstSubReg = 0;
   if (!isMoveInstr(*TRI, , SrcReg, DstReg, SrcSubReg, DstSubReg))
 return false;
   // Check if the destination of this copy has any other affinity.
@@ -3823,7 +3823,7 @@
 if ( ==  || !MI.isCopyLike() || MI.getParent() != OrigBB)
   continue;
 Register OtherSrcReg, OtherReg;
-unsigned OtherSrcSubReg, OtherSubReg;
+unsigned OtherSrcSubReg = 0, OtherSubReg = 0;
 if (!isMoveInstr(*TRI, , OtherSrcReg, OtherReg, OtherSrcSubReg,
 OtherSubReg))
   return false;
Index: clang/lib/CodeGen/CGBlocks.cpp
===
--- clang/lib/CodeGen/CGBlocks.cpp
+++ clang/lib/CodeGen/CGBlocks.cpp
@@ -2697,7 +2697,7 @@
   }
 
   bool HasByrefExtendedLayout = false;
-  Qualifiers::ObjCLifetime Lifetime;
+  Qualifiers::ObjCLifetime Lifetime = Qualifiers::OCL_None;;
   if (getContext().getByrefLifetime(Ty, Lifetime, HasByrefExtendedLayout) &&
   HasByrefExtendedLayout) {
 /// void *__byref_variable_layout;
@@ -2767,8 +2767,8 @@
   const VarDecl  = *emission.Variable;
   QualType type = D.getType();
 
-  bool HasByrefExtendedLayout;
-  Qualifiers::ObjCLifetime ByrefLifetime;
+  bool HasByrefExtendedLayout = false;
+  Qualifiers::ObjCLifetime ByrefLifetime = Qualifiers::OCL_None;
   bool ByRefHasLifetime =
 getContext().getByrefLifetime(type, ByrefLifetime, HasByrefExtendedLayout);
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D94952: [clangd] Take into account what is in the index (symbols, references, etc.) at indexes merge

[Aleksandr Platonov via Phabricator via cfe-commits]

ArcsinX added inline comments.



Comment at: clang-tools-extra/clangd/index/FileIndex.cpp:433
 PreambleSymbols.update(
-Uri, std::make_unique(std::move(*IF->Symbols)),
+FilePath ? *FilePath : (consumeError(FilePath.takeError()), Uri),
+std::make_unique(std::move(*IF->Symbols)),

ArcsinX wrote:
> sammccall wrote:
> > Is this change related? It changes the key scheme for the preamble index 
> > from URIs to paths, but I'm not sure why.
> > 
> > Do we have multiple URIs pointing to the same path? What are they 
> > concretely?
> This is the main thing in this patch. I will try to explain.
> We use these keys to create the file list, which is used by `indexedFiles()`.
> Currently, the preamble index contains URI's instead of paths (as a file 
> list), that leads to the function returned by `PreambleIndex::indexedFiles()` 
> always return `false` (because we pass to this function paths, not URI's). 
> So, we always take data from the preamble index (but maybe we should not in 
> some cases).
> 
> that leads to the function returned by `PreambleIndex::indexedFiles()` always 
> return `false` (because we pass to this function paths, not URI's)

This is a bit incorrect.
We pass to this function URI, but this URI is converted to path. i.e. 
`MemIndex::indexedFiles()`, `Dex::indexedFiles()` expect that `Files` are 
paths, but they are URI's for the preamble index. That's why 
`PreambleIndex::indexedFiles()` always return `false`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94952

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

[PATCH] D95419: [clangd] Fix filename completion at the end of file

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D95419

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


Index: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
===
--- clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -2573,7 +2573,7 @@
 }
 
 TEST(CompletionTest, IncludedCompletionKinds) {
-  Annotations Test(R"cpp(#include "^")cpp");
+  Annotations Test(R"cpp(#include "^)cpp");
   auto TU = TestTU::withCode(Test.code());
   TU.AdditionalFiles["sub/bar.h"] = "";
   TU.ExtraArgs.push_back("-I" + testPath("sub"));
Index: clang-tools-extra/clangd/CodeComplete.cpp
===
--- clang-tools-extra/clangd/CodeComplete.cpp
+++ clang-tools-extra/clangd/CodeComplete.cpp
@@ -1123,7 +1123,11 @@
   // skip all includes in this case; these completions are really simple.
   PreambleBounds PreambleRegion =
   ComputePreambleBounds(*CI->getLangOpts(), *ContentsBuffer, 0);
-  bool CompletingInPreamble = PreambleRegion.Size > Input.Offset;
+  // If preamble doesn't end on a new line, include the following byte in
+  // preamble to enable include completion on a new file. e.g: #include "^
+  bool CompletingInPreamble =
+  PreambleRegion.Size + !PreambleRegion.PreambleEndsAtStartOfLine >
+  Input.Offset;
   if (Input.Patch)
 Input.Patch->apply(*CI);
   // NOTE: we must call BeginSourceFile after prepareCompilerInstance. 
Otherwise


Index: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
===
--- clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -2573,7 +2573,7 @@
 }
 
 TEST(CompletionTest, IncludedCompletionKinds) {
-  Annotations Test(R"cpp(#include "^")cpp");
+  Annotations Test(R"cpp(#include "^)cpp");
   auto TU = TestTU::withCode(Test.code());
   TU.AdditionalFiles["sub/bar.h"] = "";
   TU.ExtraArgs.push_back("-I" + testPath("sub"));
Index: clang-tools-extra/clangd/CodeComplete.cpp
===
--- clang-tools-extra/clangd/CodeComplete.cpp
+++ clang-tools-extra/clangd/CodeComplete.cpp
@@ -1123,7 +1123,11 @@
   // skip all includes in this case; these completions are really simple.
   PreambleBounds PreambleRegion =
   ComputePreambleBounds(*CI->getLangOpts(), *ContentsBuffer, 0);
-  bool CompletingInPreamble = PreambleRegion.Size > Input.Offset;
+  // If preamble doesn't end on a new line, include the following byte in
+  // preamble to enable include completion on a new file. e.g: #include "^
+  bool CompletingInPreamble =
+  PreambleRegion.Size + !PreambleRegion.PreambleEndsAtStartOfLine >
+  Input.Offset;
   if (Input.Patch)
 Input.Patch->apply(*CI);
   // NOTE: we must call BeginSourceFile after prepareCompilerInstance. Otherwise
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D95417: [NFC] Disallow unused prefixes under clang/test/CodeGen

[Fangrui Song via Phabricator via cfe-commits]

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

Thanks!




Comment at: 
clang/test/CodeGen/catch-alignment-assumption-attribute-align_value-on-lvalue.cpp:2
+// RUN: %clang_cc1 -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck %s
 // RUN: %clang_cc1 -fsanitize=alignment -fno-sanitize-recover=alignment 
-emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck %s 
-implicit-check-not="call void @__ubsan_handle_alignment_assumption" 
--check-prefixes=CHECK,CHECK-SANITIZE,CHECK-SANITIZE-ANYRECOVER,CHECK-SANITIZE-NORECOVER,CHECK-SANITIZE-UNREACHABLE
 // RUN: %clang_cc1 -fsanitize=alignment -fsanitize-recover=alignment 
-emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck %s 
-implicit-check-not="call void @__ubsan_handle_alignment_assumption" 
--check-prefixes=CHECK,CHECK-SANITIZE,CHECK-SANITIZE-ANYRECOVER,CHECK-SANITIZE-RECOVER

Not: these tests can be generalized a bit: `-triple x86_64-linux-gnu` -> 
`-triple x86_64` (because they work for generic ELF and are not tied to Linux), 
but there is probably no reason to cause that churn since it has been committed.



Comment at: 
clang/test/CodeGen/catch-alignment-assumption-attribute-align_value-on-lvalue.cpp:3
 // RUN: %clang_cc1 -fsanitize=alignment -fno-sanitize-recover=alignment 
-emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck %s 
-implicit-check-not="call void @__ubsan_handle_alignment_assumption" 
--check-prefixes=CHECK,CHECK-SANITIZE,CHECK-SANITIZE-ANYRECOVER,CHECK-SANITIZE-NORECOVER,CHECK-SANITIZE-UNREACHABLE
 // RUN: %clang_cc1 -fsanitize=alignment -fsanitize-recover=alignment 
-emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck %s 
-implicit-check-not="call void @__ubsan_handle_alignment_assumption" 
--check-prefixes=CHECK,CHECK-SANITIZE,CHECK-SANITIZE-ANYRECOVER,CHECK-SANITIZE-RECOVER
 // RUN: %clang_cc1 -fsanitize=alignment -fsanitize-trap=alignment -emit-llvm 
%s -o - -triple x86_64-linux-gnu | FileCheck %s -implicit-check-not="call void 
@__ubsan_handle_alignment_assumption" 
--check-prefixes=CHECK,CHECK-SANITIZE,CHECK-SANITIZE-TRAP,CHECK-SANITIZE-UNREACHABLE

If the intention of `CHECK-NOSANITIZE` is to explicitly check there is no 
sanitizer variable, it should use some `-NOT:` patterns. However, sanitizer 
variables should be obvious so that can be omitted.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95417

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

[PATCH] D95417: [NFC] Disallow unused prefixes under clang/test/CodeGen

[Mircea Trofin via Phabricator via cfe-commits]

mtrofin created this revision.
mtrofin added a reviewer: MaskRay.
mtrofin requested review of this revision.
Herald added a reviewer: jdoerfert.
Herald added subscribers: cfe-commits, sstefan1.
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D95417

Files:
  
clang/test/CodeGen/catch-alignment-assumption-attribute-align_value-on-lvalue.cpp
  
clang/test/CodeGen/catch-alignment-assumption-attribute-alloc_align-on-function-variable.cpp
  
clang/test/CodeGen/catch-alignment-assumption-attribute-assume_aligned-on-function-two-params.cpp
  
clang/test/CodeGen/catch-alignment-assumption-builtin_assume_aligned-three-params-variable.cpp
  
clang/test/CodeGen/catch-alignment-assumption-builtin_assume_aligned-three-params.cpp
  
clang/test/CodeGen/catch-alignment-assumption-builtin_assume_aligned-two-params.cpp
  clang/test/CodeGen/catch-alignment-assumption-openmp.cpp
  clang/test/CodeGen/catch-implicit-integer-sign-changes-incdec.c
  clang/test/CodeGen/catch-implicit-integer-sign-changes-true-negatives.c
  clang/test/CodeGen/catch-implicit-signed-integer-truncations-incdec.c
  clang/test/CodeGen/catch-nullptr-and-nonzero-offset-blacklist.c
  clang/test/CodeGen/catch-nullptr-and-nonzero-offset-when-nullptr-is-defined.c
  clang/test/CodeGen/catch-nullptr-and-nonzero-offset.c
  clang/test/CodeGen/catch-pointer-overflow-volatile.c
  clang/test/CodeGen/catch-pointer-overflow.c
  clang/test/CodeGen/cmse-clear-return.c
  clang/test/CodeGen/lit.local.cfg

Index: clang/test/CodeGen/lit.local.cfg
===
--- /dev/null
+++ clang/test/CodeGen/lit.local.cfg
@@ -0,0 +1,9 @@
+# -*- Python -*- vim: set ft=python ts=4 sw=4 expandtab tw=79:
+from lit.llvm.subst import ToolSubst
+
+fc = ToolSubst('FileCheck', unresolved='fatal')
+# Insert this first. Then, we'll first update the blank FileCheck command; then,
+# the default substitution of FileCheck will replace it to its full path.
+config.substitutions.insert(0, (fc.regex,
+'FileCheck --allow-unused-prefixes=false'))
+
Index: clang/test/CodeGen/cmse-clear-return.c
===
--- clang/test/CodeGen/cmse-clear-return.c
+++ clang/test/CodeGen/cmse-clear-return.c
@@ -1,7 +1,7 @@
 // RUN: %clang_cc1 -triple thumbv8m.main   -O0 -mcmse -S -emit-llvm %s -o - | \
 // RUN:FileCheck %s --check-prefixes=CHECK,CHECK-LE,CHECK-LE-NOPT,CHECK-SOFT
 // RUN: %clang_cc1 -triple thumbebv8m.main -O0 -mcmse -S -emit-llvm %s -o - | \
-// RUN:FileCheck %s --check-prefixes=CHECK,CHECK-BE,CHECK-BE-NOPT,CHECK-SOFT
+// RUN:FileCheck %s --check-prefixes=CHECK,CHECK-BE,CHECK-SOFT
 // RUN: %clang_cc1 -triple thumbv8m.main   -O2 -mcmse -S -emit-llvm %s -o - | \
 // RUN:FileCheck %s --check-prefixes=CHECK,CHECK-LE,CHECK-LE-OPT,CHECK-SOFT
 // RUN: %clang_cc1 -triple thumbebv8m.main -O2 -mcmse -S -emit-llvm %s -o - | \
Index: clang/test/CodeGen/catch-pointer-overflow.c
===
--- clang/test/CodeGen/catch-pointer-overflow.c
+++ clang/test/CodeGen/catch-pointer-overflow.c
@@ -1,9 +1,9 @@
-// RUN: %clang_cc1 -x c -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck %s --check-prefixes=CHECK,CHECK-NOSANITIZE
+// RUN: %clang_cc1 -x c -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck %s
 // RUN: %clang_cc1 -x c -fsanitize=pointer-overflow -fno-sanitize-recover=pointer-overflow -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck %s -implicit-check-not="call void @__ubsan_handle_pointer_overflow" --check-prefixes=CHECK,CHECK-SANITIZE,CHECK-SANITIZE-C,CHECK-SANITIZE-ANYRECOVER,CHECK-SANITIZE-NORECOVER,CHECK-SANITIZE-UNREACHABLE
 // RUN: %clang_cc1 -x c -fsanitize=pointer-overflow -fsanitize-recover=pointer-overflow -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck %s -implicit-check-not="call void @__ubsan_handle_pointer_overflow" --check-prefixes=CHECK,CHECK-SANITIZE,CHECK-SANITIZE-C,CHECK-SANITIZE-ANYRECOVER,CHECK-SANITIZE-RECOVER
 // RUN: %clang_cc1 -x c -fsanitize=pointer-overflow -fsanitize-trap=pointer-overflow -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck %s -implicit-check-not="call void @__ubsan_handle_pointer_overflow" --check-prefixes=CHECK,CHECK-SANITIZE,CHECK-SANITIZE-C,CHECK-SANITIZE-TRAP,CHECK-SANITIZE-UNREACHABLE
 
-// RUN: %clang_cc1 -x c++ -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck %s --check-prefixes=CHECK,CHECK-NOSANITIZE
+// RUN: %clang_cc1 -x c++ -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck %s
 // RUN: %clang_cc1 -x c++ -fsanitize=pointer-overflow -fno-sanitize-recover=pointer-overflow -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck %s -implicit-check-not="call void @__ubsan_handle_pointer_overflow" --check-prefixes=CHECK,CHECK-SANITIZE,CHECK-SANITIZE-CPP,CHECK-SANITIZE-ANYRECOVER,CHECK-SANITIZE-NORECOVER,CHECK-SANITIZE-UNREACHABLE
 // RUN: %clang_cc1 -x c++ -fsanitize=pointer-overflow 

[PATCH] D94955: [clang-format] Treat ForEachMacros as loops

[Jiashu Zou via Phabricator via cfe-commits]

GoBigorGoHome added a comment.






Comment at: clang/unittests/Format/FormatTest.cpp:995
 TEST_F(FormatTest, ForEachLoops) {
   verifyFormat("void f() {\n"
+   "  foreach (Item *item, itemlist) {\n"

MyDeveloperDay wrote:
> I'd like you to assert that short loops are off
You mean make sure that `AllowShortLoopsOnASingleLine` is false?
The default style of `verifyFormat` is LLVM style where 
`AllowShortLoopsOnASingleLine` is already false.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94955

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

[PATCH] D94315: [OpenMP][FIX] Enforce a function boundary for a new data environment

[Johannes Doerfert via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGbd756286d2e7: [OpenMP][FIX] Enforce a function boundary for 
a new data environment (authored by jdoerfert).
Herald added a reviewer: bollu.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94315

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
  clang/test/OpenMP/parallel_if_codegen.cpp

Index: clang/test/OpenMP/parallel_if_codegen.cpp
===
--- clang/test/OpenMP/parallel_if_codegen.cpp
+++ clang/test/OpenMP/parallel_if_codegen.cpp
@@ -7,6 +7,7 @@
 // RUN: %clang_cc1 -fopenmp-simd -fopenmp-version=45 -x c++ -triple %itanium_abi_triple -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck --check-prefix SIMD-ONLY0 %s
 // SIMD-ONLY0-NOT: {{__kmpc|__tgt}}
 
+// RUN: %clang_cc1 -verify -fopenmp -x c++ -triple %itanium_abi_triple -emit-llvm %s -disable-O0-optnone -o - | FileCheck %s --check-prefix=WOPT
 // RUN: %clang_cc1 -verify -fopenmp -x c++ -triple %itanium_abi_triple -emit-llvm %s -o - | FileCheck %s
 // RUN: %clang_cc1 -fopenmp -x c++ -std=c++11 -triple %itanium_abi_triple -emit-pch -o %t %s
 // RUN: %clang_cc1 -fopenmp -x c++ -triple %itanium_abi_triple -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s
@@ -96,14 +97,20 @@
   return tmain(Arg);
 }
 
+// WOPT: noinline
+// WOPT-NOT: optnone
 // CHECK: define internal {{.*}}void [[CAP_FN4]]
 // CHECK: call {{.*}}void @{{.+}}fn4
 // CHECK: ret void
 
+// WOPT: noinline
+// WOPT-NOT: optnone
 // CHECK: define internal {{.*}}void [[CAP_FN5]]
 // CHECK: call {{.*}}void @{{.+}}fn5
 // CHECK: ret void
 
+// WOPT: noinline
+// WOPT-NOT: optnone
 // CHECK: define internal {{.*}}void [[CAP_FN6]]
 // CHECK: call {{.*}}void @{{.+}}fn6
 // CHECK: ret void
@@ -129,14 +136,20 @@
 // CHECK: br label %[[OMP_END]]
 // CHECK: [[OMP_END]]
 
+// WOPT: noinline
+// WOPT-NOT: optnone
 // CHECK: define internal {{.*}}void [[CAP_FN1]]
 // CHECK: call {{.*}}void @{{.+}}fn1
 // CHECK: ret void
 
+// WOPT: noinline
+// WOPT-NOT: optnone
 // CHECK: define internal {{.*}}void [[CAP_FN2]]
 // CHECK: call {{.*}}void @{{.+}}fn2
 // CHECK: ret void
 
+// WOPT: noinline
+// WOPT-NOT: optnone
 // CHECK: define internal {{.*}}void [[CAP_FN3]]
 // CHECK: call {{.*}}void @{{.+}}fn3
 // CHECK: ret void
Index: clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
===
--- clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
+++ clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
@@ -1576,11 +1576,6 @@
   auto *OutlinedFun =
   cast(CGOpenMPRuntime::emitParallelOutlinedFunction(
   D, ThreadIDVar, InnermostKind, CodeGen));
-  if (CGM.getLangOpts().Optimize) {
-OutlinedFun->removeFnAttr(llvm::Attribute::NoInline);
-OutlinedFun->removeFnAttr(llvm::Attribute::OptimizeNone);
-OutlinedFun->addFnAttr(llvm::Attribute::AlwaysInline);
-  }
   IsInTargetMasterThreadRegion = PrevIsInTargetMasterThreadRegion;
   IsInTTDRegion = PrevIsInTTDRegion;
   if (getExecutionMode() != CGOpenMPRuntimeGPU::EM_SPMD &&
@@ -1698,11 +1693,6 @@
   CodeGen.setAction(Action);
   llvm::Function *OutlinedFun = CGOpenMPRuntime::emitTeamsOutlinedFunction(
   D, ThreadIDVar, InnermostKind, CodeGen);
-  if (CGM.getLangOpts().Optimize) {
-OutlinedFun->removeFnAttr(llvm::Attribute::NoInline);
-OutlinedFun->removeFnAttr(llvm::Attribute::OptimizeNone);
-OutlinedFun->addFnAttr(llvm::Attribute::AlwaysInline);
-  }
 
   return OutlinedFun;
 }
@@ -2102,6 +2092,14 @@
   // Force inline this outlined function at its call site.
   Fn->setLinkage(llvm::GlobalValue::InternalLinkage);
 
+  // Ensure we do not inline the function. This is trivially true for the ones
+  // passed to __kmpc_fork_call but the ones calles in serialized regions
+  // could be inlined. This is not a perfect but it is closer to the invariant
+  // we want, namely, every data environment starts with a new function.
+  // TODO: We should pass the if condition to the runtime function and do the
+  //   handling there. Much cleaner code.
+  cast(OutlinedFn)->addFnAttr(llvm::Attribute::NoInline);
+
   Address ZeroAddr = CGF.CreateDefaultAlignTempAlloca(CGF.Int32Ty,
   /*Name=*/".zero.addr");
   CGF.InitTempAlloca(ZeroAddr, CGF.Builder.getInt32(/*C*/ 0));
@@ -3134,11 +3132,6 @@
   "_omp_reduction_shuffle_and_reduce_func", ());
   CGM.SetInternalFunctionAttributes(GlobalDecl(), Fn, CGFI);
   Fn->setDoesNotRecurse();
-  if (CGM.getLangOpts().Optimize) {
-Fn->removeFnAttr(llvm::Attribute::NoInline);
-Fn->removeFnAttr(llvm::Attribute::OptimizeNone);
-Fn->addFnAttr(llvm::Attribute::AlwaysInline);
-  }
 
   CodeGenFunction CGF(CGM);
   CGF.StartFunction(GlobalDecl(), C.VoidTy, Fn, CGFI, Args, Loc, Loc);
Index: 

[PATCH] D94583: [RISCV] Update V extension to v1.0-draft 08a0b464.

[Hsiangkai Wang via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGf19849a07b67: [RISCV] Update V extension to v1.0-draft 
08a0b464. (authored by HsiangKai).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94583

Files:
  clang/lib/Basic/Targets/RISCV.cpp
  clang/lib/Driver/ToolChains/Arch/RISCV.cpp
  clang/test/Driver/riscv-arch.c
  clang/test/Preprocessor/riscv-target-features.c
  llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVTargetStreamer.cpp
  llvm/test/CodeGen/RISCV/attributes.ll
  llvm/test/MC/RISCV/attribute-arch.s

Index: llvm/test/MC/RISCV/attribute-arch.s
===
--- llvm/test/MC/RISCV/attribute-arch.s
+++ llvm/test/MC/RISCV/attribute-arch.s
@@ -40,7 +40,7 @@
 # CHECK: attribute  5, "rv32i2p0_b0p93_zba0p93_zbb0p93_zbc0p93_zbe0p93_zbf0p93_zbm0p93_zbp0p93_zbr0p93_zbs0p93_zbt0p93"
 
 .attribute arch, "rv32iv"
-# CHECK: attribute  5, "rv32i2p0_v0p9"
+# CHECK: attribute  5, "rv32i2p0_v1p0"
 
 .attribute arch, "rv32izba"
 # CHECK: attribute  5, "rv32i2p0_zba0p93"
@@ -79,7 +79,7 @@
 # CHECK: attribute  5, "rv32i2p0_f2p0_zfh0p1"
 
 .attribute arch, "rv32ivzvamo_zvlsseg"
-# CHECK: attribute  5, "rv32i2p0_v0p9_zvamo0p9_zvlsseg0p9"
+# CHECK: attribute  5, "rv32i2p0_v1p0_zvamo1p0_zvlsseg1p0"
 
-.attribute arch, "rv32iv_zvamo0p9_zvlsseg"
-# CHECK: attribute  5, "rv32i2p0_v0p9_zvamo0p9_zvlsseg0p9"
+.attribute arch, "rv32iv_zvamo1p0_zvlsseg"
+# CHECK: attribute  5, "rv32i2p0_v1p0_zvamo1p0_zvlsseg1p0"
Index: llvm/test/CodeGen/RISCV/attributes.ll
===
--- llvm/test/CodeGen/RISCV/attributes.ll
+++ llvm/test/CodeGen/RISCV/attributes.ll
@@ -47,7 +47,7 @@
 ; RV32D: .attribute 5, "rv32i2p0_f2p0_d2p0"
 ; RV32C: .attribute 5, "rv32i2p0_c2p0"
 ; RV32B: .attribute 5, "rv32i2p0_b0p93_zba0p93_zbb0p93_zbc0p93_zbe0p93_zbf0p93_zbm0p93_zbp0p93_zbr0p93_zbs0p93_zbt0p93"
-; RV32V: .attribute 5, "rv32i2p0_v0p9_zvamo0p9_zvlsseg0p9"
+; RV32V: .attribute 5, "rv32i2p0_v1p0_zvamo1p0_zvlsseg1p0"
 ; RV32ZFH: .attribute 5, "rv32i2p0_f2p0_zfh0p1"
 ; RV32ZBA: .attribute 5, "rv32i2p0_zba0p93"
 ; RV32ZBB: .attribute 5, "rv32i2p0_zbb0p93"
@@ -60,7 +60,7 @@
 ; RV32ZBR: .attribute 5, "rv32i2p0_zbr0p93"
 ; RV32ZBS: .attribute 5, "rv32i2p0_zbs0p93"
 ; RV32ZBT: .attribute 5, "rv32i2p0_zbt0p93"
-; RV32COMBINED: .attribute 5, "rv32i2p0_f2p0_v0p9_zfh0p1_zbb0p93_zvamo0p9_zvlsseg0p9"
+; RV32COMBINED: .attribute 5, "rv32i2p0_f2p0_v1p0_zfh0p1_zbb0p93_zvamo1p0_zvlsseg1p0"
 
 ; RV64M: .attribute 5, "rv64i2p0_m2p0"
 ; RV64A: .attribute 5, "rv64i2p0_a2p0"
@@ -80,8 +80,8 @@
 ; RV64ZBR: .attribute 5, "rv64i2p0_zbr0p93"
 ; RV64ZBS: .attribute 5, "rv64i2p0_zbs0p93"
 ; RV64ZBT: .attribute 5, "rv64i2p0_zbt0p93"
-; RV64V: .attribute 5, "rv64i2p0_v0p9_zvamo0p9_zvlsseg0p9"
-; RV64COMBINED: .attribute 5, "rv64i2p0_f2p0_v0p9_zfh0p1_zbb0p93_zvamo0p9_zvlsseg0p9"
+; RV64V: .attribute 5, "rv64i2p0_v1p0_zvamo1p0_zvlsseg1p0"
+; RV64COMBINED: .attribute 5, "rv64i2p0_f2p0_v1p0_zfh0p1_zbb0p93_zvamo1p0_zvlsseg1p0"
 
 
 define i32 @addi(i32 %a) {
Index: llvm/lib/Target/RISCV/MCTargetDesc/RISCVTargetStreamer.cpp
===
--- llvm/lib/Target/RISCV/MCTargetDesc/RISCVTargetStreamer.cpp
+++ llvm/lib/Target/RISCV/MCTargetDesc/RISCVTargetStreamer.cpp
@@ -63,7 +63,7 @@
   if (STI.hasFeature(RISCV::FeatureStdExtB))
 Arch += "_b0p93";
   if (STI.hasFeature(RISCV::FeatureStdExtV))
-Arch += "_v0p9";
+Arch += "_v1p0";
   if (STI.hasFeature(RISCV::FeatureExtZfh))
 Arch += "_zfh0p1";
   if (STI.hasFeature(RISCV::FeatureExtZba))
@@ -89,9 +89,9 @@
   if (STI.hasFeature(RISCV::FeatureExtZbt))
 Arch += "_zbt0p93";
   if (STI.hasFeature(RISCV::FeatureExtZvamo))
-Arch += "_zvamo0p9";
+Arch += "_zvamo1p0";
   if (STI.hasFeature(RISCV::FeatureStdExtZvlsseg))
-Arch += "_zvlsseg0p9";
+Arch += "_zvlsseg1p0";
 
   emitTextAttribute(RISCVAttrs::ARCH, Arch);
 }
Index: llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
===
--- llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
+++ llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
@@ -2126,7 +2126,7 @@
   if (getFeatureBits(RISCV::FeatureStdExtB))
 formalArchStr = (Twine(formalArchStr) + "_b0p93").str();
   if (getFeatureBits(RISCV::FeatureStdExtV))
-formalArchStr = (Twine(formalArchStr) + "_v0p9").str();
+formalArchStr = (Twine(formalArchStr) + "_v1p0").str();
   if (getFeatureBits(RISCV::FeatureExtZfh))
 formalArchStr = (Twine(formalArchStr) + "_zfh0p1").str();
   if (getFeatureBits(RISCV::FeatureExtZba))
@@ -2152,9 +2152,9 @@
   if (getFeatureBits(RISCV::FeatureExtZbt))
 

[PATCH] D85474: Add -fbinutils-version= to gate ELF features on the specified binutils version

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:4642
+int Num;
+if (V == "future")
+  A->render(Args, CmdArgs);

rsmith wrote:
> dblaikie wrote:
> > ro wrote:
> > > MaskRay wrote:
> > > > phosek wrote:
> > > > > Another option would be `unstable`.
> > > > I picked `future` because there is an precedent: `-mcpu=future` is used 
> > > > by some ppc folks.
> > > I fear that's a terrible precedent: this name had to be chosen because 
> > > for some unknown, but certainly silly, reason, IBM didn't wan't to call 
> > > it `power10` before release.
> > A little more respect for other developers would be great here - hard to 
> > know it's silly if you don't know the reason & even if you think it is 
> > silly, folks may not have the same goals/requirements you do.
> > 
> > (that's not to dismiss the concern that the precedent for naming -mcpu may 
> > not apply as well here (not suggesting it doesn't either - I have no 
> > opinion either way))
> How about `latest`? (As in, "I promise I have at least the latest version of 
> binutils that you've heard of")
(offline discussion Richard is happy with 'none')


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85474

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

[PATCH] D93260: Frontend: Simplify handling of non-seeking streams in CompilerInstance, NFC

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

dexonsmith added a comment.

Ping!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93260

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

[PATCH] D93248: Frontend: Fix layering between create{,Default}OutputFile, NFC

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

dexonsmith added a comment.

Ping!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93248

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

[PATCH] D93249: Frontend: Fix memory leak in CompilerInstance::setVerboseOutputStream

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

dexonsmith added a comment.

Ping!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93249

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

[PATCH] D94745: [OpenMP][deviceRTLs] Build the deviceRTLs with OpenMP instead of target dependent language

[Jon Chesterfield via Phabricator via cfe-commits]

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

LGTM! Thank you for iterating on this, and for trying to keep amdgpu working.

With this patch, the devicertl no longer needs the cuda sdk installed to 
compile. I believe there are still some tests in the cmake that need to go 
before builds will succeed on a machine without cuda installed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94745

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

[PATCH] D94987: DR39: Perform ambiguous subobject checks for class member access as part of object argument conversion, not as part of name lookup.

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

> You can. The rule is that you resolve using-declarations to the declarations 
> they name, and you resolve typedef declarations to the types they name, and 
> the resulting set of declarations and types must be the same in every 
> subobject in which there are (non-shadowed) results -- if not, the lookup is 
> ill-formed. (Put another way, the set of entities found in each subobject 
> must be the same, but the set of declarations found in each subobject can 
> differ.)

Okay, makes sense.  So the DeclAccessPairs in the lookup result are basically 
just the first+best pair we found for any particular entity?




Comment at: clang/include/clang/AST/CXXInheritance.h:77
 
-  CXXBasePath() = default;
+  /// Additional data stashed on the base path by its consumers.
+  union {

rsmith wrote:
> rjmccall wrote:
> > Is there a way to know which case we're in, or do different consumers do 
> > different things?
> We could use a discriminated union here. Ultimately, I think what we really 
> want is to provide some opaque storage for the consumer to use, rather than 
> to have some hard-coded list here. I haven't thought of a good way to expose 
> that; I don't think it's worth templating the entire base path finding 
> mechanism to add such storage, and a single pointer isn't large enough for a 
> `DeclContext::lookup_result`.
> 
> Vassil's https://reviews.llvm.org/D91524 will make the `lookup_result` case 
> fit into a single pointer; once we adopt that, perhaps we can switch to 
> holding an opaque `void*` here?
Yeah, that might be cleaner, assuming we really need this.  What are the 
clients that need to store something specifically in the `CXXBasePath` object 
and can't just keep it separate?



Comment at: clang/lib/Sema/SemaAccess.cpp:983
+  }
+  return false;
+};

rsmith wrote:
> rjmccall wrote:
> > Is this not doing a *lot* of extra work?  I suppose this is only in the 
> > slow path where we weren't able to immediately recognize that the found 
> > decl is public or we're in a scope with obviously adequate access?
> Compared to the `isDerivedFrom` check, this will be doing one extra decl 
> context hash table lookup per class we consider on each path. I haven't 
> measured the extra work here; I can if you're concerned.
> 
> I thought for a while about better ways to handle this, and I'm not sure 
> there's a good middle ground between redoing the work like this and saving 
> the base paths from name lookup and using them here. If you have better 
> ideas, I'd appreciate them!
> 
> I suppose we probably have a spare bit on `DeclAccessPair` that we could use 
> to track whether we're in the "hard" (found in classes of different types) 
> case, and we could use `isDerivedFrom` in the "easy" cases. Do you think 
> that's worthwhile?
> 
> Ah, I see you suggest doing something like that below. Yes, I can give that a 
> go.
Thanks.  Yeah, I think it would be nice if the very common case of finding 
something in a unique class didn't have to redo any lookups.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94987

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

[PATCH] D94987: DR39: Perform ambiguous subobject checks for class member access as part of object argument conversion, not as part of name lookup.

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

In D94987#2507481 , @rjmccall wrote:

> How does this new rule work if we find overlapping but non-equal sets of 
> declarations in different subobjects?  I'm sure you can get that with `using` 
> declarations.

You can. The rule is that you resolve using-declarations to the declarations 
they name, and you resolve typedef declarations to the types they name, and the 
resulting set of declarations and types must be the same in every subobject in 
which there are (non-shadowed) results -- if not, the lookup is ill-formed. 
(Put another way, the set of entities found in each subobject must be the same, 
but the set of declarations found in each subobject can differ.)




Comment at: clang/include/clang/AST/CXXInheritance.h:77
 
-  CXXBasePath() = default;
+  /// Additional data stashed on the base path by its consumers.
+  union {

rjmccall wrote:
> Is there a way to know which case we're in, or do different consumers do 
> different things?
We could use a discriminated union here. Ultimately, I think what we really 
want is to provide some opaque storage for the consumer to use, rather than to 
have some hard-coded list here. I haven't thought of a good way to expose that; 
I don't think it's worth templating the entire base path finding mechanism to 
add such storage, and a single pointer isn't large enough for a 
`DeclContext::lookup_result`.

Vassil's https://reviews.llvm.org/D91524 will make the `lookup_result` case fit 
into a single pointer; once we adopt that, perhaps we can switch to holding an 
opaque `void*` here?



Comment at: clang/lib/Sema/SemaAccess.cpp:891
+// Note that declaresSameEntity doesn't correctly determine whether
+// two type declarations declare the same type entity.
+if ((!Best || Best->getAccess() > Corresponding->getAccess()) &&

rjmccall wrote:
> Is it okay to check them in sequence like this, or do we need to check in 
> advance which case to use?
For types, `declaresSameEntity` returns true for a subset of the cases for 
which `declaresSameType` returns true, so it will always be correct to check 
both (though for a type, checking `declaresSameEntity` will always be 
redundant).



Comment at: clang/lib/Sema/SemaAccess.cpp:983
+  }
+  return false;
+};

rjmccall wrote:
> Is this not doing a *lot* of extra work?  I suppose this is only in the slow 
> path where we weren't able to immediately recognize that the found decl is 
> public or we're in a scope with obviously adequate access?
Compared to the `isDerivedFrom` check, this will be doing one extra decl 
context hash table lookup per class we consider on each path. I haven't 
measured the extra work here; I can if you're concerned.

I thought for a while about better ways to handle this, and I'm not sure 
there's a good middle ground between redoing the work like this and saving the 
base paths from name lookup and using them here. If you have better ideas, I'd 
appreciate them!

I suppose we probably have a spare bit on `DeclAccessPair` that we could use to 
track whether we're in the "hard" (found in classes of different types) case, 
and we could use `isDerivedFrom` in the "easy" cases. Do you think that's 
worthwhile?

Ah, I see you suggest doing something like that below. Yes, I can give that a 
go.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94987

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

[PATCH] D94745: [OpenMP][WIP] Build the deviceRTLs with OpenMP instead of target dependent language - NOT FOR REVIEW

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 updated this revision to Diff 319180.
tianshilei1992 added a comment.
Herald added a subscriber: jvesely.

- Fixed CMake error on CMake 3.16 or lower version as `ZIP_LISTS` doesn't work;
- Fixed (hopefully) compilation break on AMDGCN by gaurding `allocator.h` with 
macro.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94745

Files:
  clang/lib/Driver/ToolChains/Cuda.cpp
  clang/test/Driver/Inputs/libomptarget/libomptarget-nvptx-cuda_80-sm_20.bc
  clang/test/Driver/Inputs/libomptarget/libomptarget-nvptx-sm_20.bc
  clang/test/Driver/openmp-offload-gpu.c
  openmp/libomptarget/deviceRTLs/amdgcn/src/target_impl.h
  openmp/libomptarget/deviceRTLs/common/allocator.h
  openmp/libomptarget/deviceRTLs/common/omptarget.h
  openmp/libomptarget/deviceRTLs/common/src/omp_data.cu
  openmp/libomptarget/deviceRTLs/common/src/reduction.cu
  openmp/libomptarget/deviceRTLs/nvptx/CMakeLists.txt
  openmp/libomptarget/deviceRTLs/nvptx/src/nvptx_interface.h
  openmp/libomptarget/deviceRTLs/nvptx/src/target_impl.cu
  openmp/libomptarget/deviceRTLs/nvptx/src/target_impl.h

Index: openmp/libomptarget/deviceRTLs/nvptx/src/target_impl.h
===
--- openmp/libomptarget/deviceRTLs/nvptx/src/target_impl.h
+++ openmp/libomptarget/deviceRTLs/nvptx/src/target_impl.h
@@ -13,18 +13,16 @@
 #define _TARGET_IMPL_H_
 
 #include 
-#include 
 #include 
 #include 
 #include 
 
 #include "nvptx_interface.h"
 
-#define DEVICE __device__
-#define INLINE __forceinline__ DEVICE
-#define NOINLINE __noinline__ DEVICE
-#define SHARED __shared__
-#define ALIGN(N) __align__(N)
+#define DEVICE
+#define INLINE inline __attribute__((always_inline))
+#define NOINLINE __attribute__((noinline))
+#define ALIGN(N) __attribute__((aligned(N)))
 
 
 // Kernel options
@@ -96,10 +94,6 @@
 INLINE uint32_t __kmpc_impl_ffs(uint32_t x) { return __builtin_ffs(x); }
 INLINE uint32_t __kmpc_impl_popc(uint32_t x) { return __builtin_popcount(x); }
 
-#ifndef CUDA_VERSION
-#error CUDA_VERSION macro is undefined, something wrong with cuda.
-#endif
-
 DEVICE __kmpc_impl_lanemask_t __kmpc_impl_activemask();
 
 DEVICE int32_t __kmpc_impl_shfl_sync(__kmpc_impl_lanemask_t Mask, int32_t Var,
Index: openmp/libomptarget/deviceRTLs/nvptx/src/target_impl.cu
===
--- openmp/libomptarget/deviceRTLs/nvptx/src/target_impl.cu
+++ openmp/libomptarget/deviceRTLs/nvptx/src/target_impl.cu
@@ -14,8 +14,6 @@
 #include "target_impl.h"
 #include "common/debug.h"
 
-#include 
-
 DEVICE void __kmpc_impl_unpack(uint64_t val, uint32_t , uint32_t ) {
   asm volatile("mov.b64 {%0,%1}, %2;" : "=r"(lo), "=r"(hi) : "l"(val));
 }
Index: openmp/libomptarget/deviceRTLs/nvptx/src/nvptx_interface.h
===
--- openmp/libomptarget/deviceRTLs/nvptx/src/nvptx_interface.h
+++ openmp/libomptarget/deviceRTLs/nvptx/src/nvptx_interface.h
@@ -11,7 +11,8 @@
 
 #include 
 
-#define EXTERN extern "C" __device__
+#define EXTERN extern "C"
+
 typedef uint32_t __kmpc_impl_lanemask_t;
 typedef uint32_t omp_lock_t; /* arbitrary type of the right length */
 
Index: openmp/libomptarget/deviceRTLs/nvptx/CMakeLists.txt
===
--- openmp/libomptarget/deviceRTLs/nvptx/CMakeLists.txt
+++ openmp/libomptarget/deviceRTLs/nvptx/CMakeLists.txt
@@ -10,6 +10,21 @@
 #
 ##===--===##
 
+# TODO: This part needs to be refined when libomptarget is going to support
+# Windows!
+# TODO: This part can also be removed if we can change the clang driver to make
+# it support device only compilation.
+if(CMAKE_HOST_SYSTEM_PROCESSOR MATCHES "x86_64")
+  set(aux_triple x86_64-unknown-linux-gnu)
+elseif(CMAKE_HOST_SYSTEM_PROCESSOR MATCHES "ppc64le")
+  set(aux_triple powerpc64le-unknown-linux-gnu)
+elseif(CMAKE_HOST_SYSTEM_PROCESSOR MATCHES "aarch64")
+  set(aux_triple aarch64-unknown-linux-gnu)
+else()
+  libomptarget_say("Not building CUDA offloading device RTL: unknown host arch: ${CMAKE_HOST_SYSTEM_PROCESSOR}")
+  return()
+endif()
+
 get_filename_component(devicertl_base_directory
   ${CMAKE_CURRENT_SOURCE_DIR}
   DIRECTORY)
@@ -79,61 +94,91 @@
 )
 
 # Set flags for LLVM Bitcode compilation.
-set(bc_flags ${LIBOMPTARGET_NVPTX_SELECTED_CUDA_COMPILER_FLAGS}
+set(bc_flags -S -x c++
+ -target nvptx64
+ -Xclang -emit-llvm-bc
+ -Xclang -aux-triple -Xclang ${aux_triple}
+ -fopenmp -fopenmp-cuda-mode -Xclang -fopenmp-is-device
+ -D__CUDACC__
  -I${devicertl_base_directory}
  -I${devicertl_nvptx_directory}/src)
 
 if(${LIBOMPTARGET_NVPTX_DEBUG})
-  

[PATCH] D94745: [OpenMP][WIP] Build the deviceRTLs with OpenMP instead of target dependent language - NOT FOR REVIEW

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield added inline comments.



Comment at: openmp/libomptarget/deviceRTLs/common/allocator.h:16
+
+// Follows the pattern in interface.h
+// Clang sema checks this type carefully, needs to closely match that from 
omp.h

If we guard this with #ifdef _OPENMP, and we change amdgcn/src/target_impl.h 
from

`#define SHARED __attribute__((shared))`
to
`#define SHARED(NAME) __attribute__((shared)) NAME`
`#define EXTERN_SHARED(NAME) __attribute__((shared)) NAME`

then there's a credible chance the downstream amdgpu hip build will continue 
working


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94745

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

[PATCH] D91297: Frontend: Take VFS and MainFileBuffer by reference in PrecompiledPreamble::CanReuse, NFC

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

dexonsmith added a comment.

Thanks for the reviews; pushed f4d02fbe418db55375b78b8f57e47126e4642fb6 
.




Comment at: clang/include/clang/Frontend/PrecompiledPreamble.h:108
   bool CanReuse(const CompilerInvocation ,
-const llvm::MemoryBuffer *MainFileBuffer, PreambleBounds 
Bounds,
-llvm::vfs::FileSystem *VFS) const;
+const llvm::MemoryBufferRef ,
+PreambleBounds Bounds, llvm::vfs::FileSystem ) const;

kadircet wrote:
> dexonsmith wrote:
> > jansvoboda11 wrote:
> > > dexonsmith wrote:
> > > > kadircet wrote:
> > > > > why not accept a value directly here? (i.e. drop const and ref)
> > > > Ah, yes, I've done this a few times, and it still seems not quite 
> > > > right. But the alternative also doesn't feel right when it's not 
> > > > necessary:
> > > > ```
> > > > #include "llvm/Basic/MemoryBufferRef.h"
> > > > ```
> > > > I'm happy either way since that file is fairly careful to avoid 
> > > > bloating includes.
> > > I agree this looks a bit odd, but avoiding an unnecessary include seems 
> > > like a good excuse.
> > @kadircet , WDYT?
> sorry i was on vacation and just got the chance to get back to this.
> 
> I don't feel so bad about the include, as the header itself is small-ish and 
> only includes StringRef.h, which is already included by this header. So I 
> would lean towards accepting this by value and keeping the API clean, rather 
> than trying to get away with a forward declaration.
> 
> but definitely up to you, i don't feel strongly about it in any case. (as you 
> can easily make the argument of header for MemoryBufferRef getting bloated 
> over time)
I decided to leave it as-is for now to match the other `MemoryBufferRef` APIs 
in the file... but I take your point that `MemoryBufferRef` is a cheap include 
and I expect it will stay cheap. I'll aim to circle back and update the whole 
file at some point.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91297

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

[PATCH] D91297: Frontend: Take VFS and MainFileBuffer by reference in PrecompiledPreamble::CanReuse, NFC

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rGf4d02fbe418d: Frontend: Take VFS and MainFileBuffer by 
reference in PrecompiledPreamble… (authored by dexonsmith).
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91297

Files:
  clang-tools-extra/clangd/Preamble.cpp
  clang/include/clang/Frontend/PrecompiledPreamble.h
  clang/lib/Frontend/ASTUnit.cpp
  clang/lib/Frontend/PrecompiledPreamble.cpp

Index: clang/lib/Frontend/PrecompiledPreamble.cpp
===
--- clang/lib/Frontend/PrecompiledPreamble.cpp
+++ clang/lib/Frontend/PrecompiledPreamble.cpp
@@ -501,12 +501,12 @@
 }
 
 bool PrecompiledPreamble::CanReuse(const CompilerInvocation ,
-   const llvm::MemoryBuffer *MainFileBuffer,
+   const llvm::MemoryBufferRef ,
PreambleBounds Bounds,
-   llvm::vfs::FileSystem *VFS) const {
+   llvm::vfs::FileSystem ) const {
 
   assert(
-  Bounds.Size <= MainFileBuffer->getBufferSize() &&
+  Bounds.Size <= MainFileBuffer.getBufferSize() &&
   "Buffer is too large. Bounds were calculated from a different buffer?");
 
   auto PreambleInvocation = std::make_shared(Invocation);
@@ -520,7 +520,7 @@
   if (PreambleBytes.size() != Bounds.Size ||
   PreambleEndsAtStartOfLine != Bounds.PreambleEndsAtStartOfLine ||
   !std::equal(PreambleBytes.begin(), PreambleBytes.end(),
-  MainFileBuffer->getBuffer().begin()))
+  MainFileBuffer.getBuffer().begin()))
 return false;
   // The preamble has not changed. We may be able to re-use the precompiled
   // preamble.
@@ -532,14 +532,14 @@
   llvm::StringSet<> OverriddenAbsPaths; // Either by buffers or files.
   for (const auto  : PreprocessorOpts.RemappedFiles) {
 llvm::vfs::Status Status;
-if (!moveOnNoError(VFS->status(R.second), Status)) {
+if (!moveOnNoError(VFS.status(R.second), Status)) {
   // If we can't stat the file we're remapping to, assume that something
   // horrible happened.
   return false;
 }
 // If a mapped file was previously missing, then it has changed.
 llvm::SmallString<128> MappedPath(R.first);
-if (!VFS->makeAbsolute(MappedPath))
+if (!VFS.makeAbsolute(MappedPath))
   OverriddenAbsPaths.insert(MappedPath);
 
 OverriddenFiles[Status.getUniqueID()] = PreambleFileHash::createForFile(
@@ -552,13 +552,13 @@
 const PrecompiledPreamble::PreambleFileHash PreambleHash =
 PreambleFileHash::createForMemoryBuffer(RB.second->getMemBufferRef());
 llvm::vfs::Status Status;
-if (moveOnNoError(VFS->status(RB.first), Status))
+if (moveOnNoError(VFS.status(RB.first), Status))
   OverriddenFiles[Status.getUniqueID()] = PreambleHash;
 else
   OverridenFileBuffers[RB.first] = PreambleHash;
 
 llvm::SmallString<128> MappedPath(RB.first);
-if (!VFS->makeAbsolute(MappedPath))
+if (!VFS.makeAbsolute(MappedPath))
   OverriddenAbsPaths.insert(MappedPath);
   }
 
@@ -574,7 +574,7 @@
 }
 
 llvm::vfs::Status Status;
-if (!moveOnNoError(VFS->status(F.first()), Status)) {
+if (!moveOnNoError(VFS.status(F.first()), Status)) {
   // If the file's buffer is not remapped and we can't stat it,
   // assume that something horrible happened.
   return false;
@@ -603,7 +603,7 @@
   return false;
 // If a file previously recorded as missing exists as a regular file, then
 // consider the preamble out-of-date.
-if (auto Status = VFS->status(F.getKey())) {
+if (auto Status = VFS.status(F.getKey())) {
   if (Status->isRegularFile())
 return false;
 }
Index: clang/lib/Frontend/ASTUnit.cpp
===
--- clang/lib/Frontend/ASTUnit.cpp
+++ clang/lib/Frontend/ASTUnit.cpp
@@ -1317,8 +1317,8 @@
 return nullptr;
 
   if (Preamble) {
-if (Preamble->CanReuse(PreambleInvocationIn, MainFileBuffer.get(), Bounds,
-   VFS.get())) {
+if (Preamble->CanReuse(PreambleInvocationIn, *MainFileBuffer, Bounds,
+   *VFS)) {
   // Okay! We can re-use the precompiled preamble.
 
   // Set the state of the diagnostic object to mimic its state
Index: clang/include/clang/Frontend/PrecompiledPreamble.h
===
--- clang/include/clang/Frontend/PrecompiledPreamble.h
+++ clang/include/clang/Frontend/PrecompiledPreamble.h
@@ -105,8 +105,8 @@
   /// Check whether PrecompiledPreamble can be reused for the new contents(\p
   /// MainFileBuffer) of the main file.
   bool CanReuse(const CompilerInvocation ,
-const llvm::MemoryBuffer *MainFileBuffer, 

[PATCH] D91913: Suppress non-conforming GNU paste extension in all standard-conforming modes

[dmajor via Phabricator via cfe-commits]

dmajor added a comment.

Our bots are green at b43c26d036dc 
. Many 
thanks @hvdijk for the quick response.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91913

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

[PATCH] D94961: [OpenMP] Add OpenMP offloading toolchain for AMDGPU

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield added a comment.

The deviceRTL is changing from cuda/hip to openmp at present. It would be good 
to be able to compile that as openmp for amdgpu, which needs a patch roughly 
like this and probably some follow up.

It's plausible that the contents of this file are only really of interest to 
AMD people, in which case we should probably land it in order to iterate in 
tree instead of downstream. It now has the cmake hook and a proof of concept 
test case.

@abataev @grokos @jdoerfert your guidance would be very welcome, but equally if 
you'd prefer leave us to sink or swim that's completely reasonable too.

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94961

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

[PATCH] D94745: [OpenMP][WIP] Build the deviceRTLs with OpenMP instead of target dependent language - NOT FOR REVIEW

[Shilei Tian via Phabricator via cfe-commits]

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

Fixed comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94745

Files:
  clang/lib/Driver/ToolChains/Cuda.cpp
  clang/test/Driver/Inputs/libomptarget/libomptarget-nvptx-cuda_80-sm_20.bc
  clang/test/Driver/Inputs/libomptarget/libomptarget-nvptx-sm_20.bc
  clang/test/Driver/openmp-offload-gpu.c
  openmp/libomptarget/deviceRTLs/common/allocator.h
  openmp/libomptarget/deviceRTLs/common/omptarget.h
  openmp/libomptarget/deviceRTLs/common/src/omp_data.cu
  openmp/libomptarget/deviceRTLs/common/src/reduction.cu
  openmp/libomptarget/deviceRTLs/nvptx/CMakeLists.txt
  openmp/libomptarget/deviceRTLs/nvptx/src/nvptx_interface.h
  openmp/libomptarget/deviceRTLs/nvptx/src/target_impl.cu
  openmp/libomptarget/deviceRTLs/nvptx/src/target_impl.h

Index: openmp/libomptarget/deviceRTLs/nvptx/src/target_impl.h
===
--- openmp/libomptarget/deviceRTLs/nvptx/src/target_impl.h
+++ openmp/libomptarget/deviceRTLs/nvptx/src/target_impl.h
@@ -13,18 +13,16 @@
 #define _TARGET_IMPL_H_
 
 #include 
-#include 
 #include 
 #include 
 #include 
 
 #include "nvptx_interface.h"
 
-#define DEVICE __device__
-#define INLINE __forceinline__ DEVICE
-#define NOINLINE __noinline__ DEVICE
-#define SHARED __shared__
-#define ALIGN(N) __align__(N)
+#define DEVICE
+#define INLINE inline __attribute__((always_inline))
+#define NOINLINE __attribute__((noinline))
+#define ALIGN(N) __attribute__((aligned(N)))
 
 
 // Kernel options
@@ -96,10 +94,6 @@
 INLINE uint32_t __kmpc_impl_ffs(uint32_t x) { return __builtin_ffs(x); }
 INLINE uint32_t __kmpc_impl_popc(uint32_t x) { return __builtin_popcount(x); }
 
-#ifndef CUDA_VERSION
-#error CUDA_VERSION macro is undefined, something wrong with cuda.
-#endif
-
 DEVICE __kmpc_impl_lanemask_t __kmpc_impl_activemask();
 
 DEVICE int32_t __kmpc_impl_shfl_sync(__kmpc_impl_lanemask_t Mask, int32_t Var,
Index: openmp/libomptarget/deviceRTLs/nvptx/src/target_impl.cu
===
--- openmp/libomptarget/deviceRTLs/nvptx/src/target_impl.cu
+++ openmp/libomptarget/deviceRTLs/nvptx/src/target_impl.cu
@@ -14,8 +14,6 @@
 #include "target_impl.h"
 #include "common/debug.h"
 
-#include 
-
 DEVICE void __kmpc_impl_unpack(uint64_t val, uint32_t , uint32_t ) {
   asm volatile("mov.b64 {%0,%1}, %2;" : "=r"(lo), "=r"(hi) : "l"(val));
 }
Index: openmp/libomptarget/deviceRTLs/nvptx/src/nvptx_interface.h
===
--- openmp/libomptarget/deviceRTLs/nvptx/src/nvptx_interface.h
+++ openmp/libomptarget/deviceRTLs/nvptx/src/nvptx_interface.h
@@ -11,7 +11,8 @@
 
 #include 
 
-#define EXTERN extern "C" __device__
+#define EXTERN extern "C"
+
 typedef uint32_t __kmpc_impl_lanemask_t;
 typedef uint32_t omp_lock_t; /* arbitrary type of the right length */
 
Index: openmp/libomptarget/deviceRTLs/nvptx/CMakeLists.txt
===
--- openmp/libomptarget/deviceRTLs/nvptx/CMakeLists.txt
+++ openmp/libomptarget/deviceRTLs/nvptx/CMakeLists.txt
@@ -10,6 +10,21 @@
 #
 ##===--===##
 
+# TODO: This part needs to be refined when libomptarget is going to support
+# Windows!
+# TODO: This part can also be removed if we can change the clang driver to make
+# it support device only compilation.
+if(CMAKE_HOST_SYSTEM_PROCESSOR MATCHES "x86_64")
+  set(aux_triple x86_64-unknown-linux-gnu)
+elseif(CMAKE_HOST_SYSTEM_PROCESSOR MATCHES "ppc64le")
+  set(aux_triple powerpc64le-unknown-linux-gnu)
+elseif(CMAKE_HOST_SYSTEM_PROCESSOR MATCHES "aarch64")
+  set(aux_triple aarch64-unknown-linux-gnu)
+else()
+  libomptarget_say("Not building CUDA offloading device RTL: unknown host arch: ${CMAKE_HOST_SYSTEM_PROCESSOR}")
+  return()
+endif()
+
 get_filename_component(devicertl_base_directory
   ${CMAKE_CURRENT_SOURCE_DIR}
   DIRECTORY)
@@ -79,61 +94,82 @@
 )
 
 # Set flags for LLVM Bitcode compilation.
-set(bc_flags ${LIBOMPTARGET_NVPTX_SELECTED_CUDA_COMPILER_FLAGS}
+set(bc_flags -S -x c++
+ -target nvptx64
+ -Xclang -emit-llvm-bc
+ -Xclang -aux-triple -Xclang ${aux_triple}
+ -fopenmp -fopenmp-cuda-mode -Xclang -fopenmp-is-device
+ -D__CUDACC__
  -I${devicertl_base_directory}
  -I${devicertl_nvptx_directory}/src)
 
 if(${LIBOMPTARGET_NVPTX_DEBUG})
-  set(bc_flags ${bc_flags} -DOMPTARGET_NVPTX_DEBUG=-1)
+  list(APPEND bc_flags -DOMPTARGET_NVPTX_DEBUG=-1)
 else()
-  set(bc_flags ${bc_flags} -DOMPTARGET_NVPTX_DEBUG=0)
+  list(APPEND bc_flags -DOMPTARGET_NVPTX_DEBUG=0)
 endif()

[PATCH] D85474: Add -fbinutils-version= to gate ELF features on the specified binutils version

[Fangrui Song via Phabricator via cfe-commits]

MaskRay updated this revision to Diff 319174.
MaskRay marked 8 inline comments as done.
MaskRay edited the summary of this revision.
MaskRay added a comment.

Address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85474

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/CodeGenOptions.h
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/Driver/fbinutils-version.c
  llvm/include/llvm/MC/MCAsmInfo.h
  llvm/include/llvm/Target/TargetMachine.h
  llvm/include/llvm/Target/TargetOptions.h
  llvm/lib/CodeGen/LLVMTargetMachine.cpp
  llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
  llvm/lib/Target/TargetMachine.cpp
  llvm/test/CodeGen/X86/explicit-section-mergeable.ll
  llvm/tools/llc/llc.cpp

Index: llvm/tools/llc/llc.cpp
===
--- llvm/tools/llc/llc.cpp
+++ llvm/tools/llc/llc.cpp
@@ -82,6 +82,15 @@
  cl::value_desc("N"),
  cl::desc("Repeat compilation N times for timing"));
 
+static cl::opt
+BinutilsVersion("binutils-version", cl::Hidden,
+cl::desc("Produced object files can use all ELF features "
+ "supported by this binutils version and newer."
+ "If -no-integrated-as is specified, the generated "
+ "assembly will consider GNU as support."
+ "'none' means that all ELF features can be used, "
+ "regardless of binutils support"));
+
 static cl::opt
 NoIntegratedAssembler("no-integrated-as", cl::Hidden,
   cl::desc("Disable integrated assembler"));
@@ -430,6 +439,8 @@
   TargetOptions Options;
   auto InitializeOptions = [&](const Triple ) {
 Options = codegen::InitTargetOptionsFromCodeGenFlags(TheTriple);
+Options.BinutilsVersion =
+TargetMachine::parseBinutilsVersion(BinutilsVersion);
 Options.DisableIntegratedAS = NoIntegratedAssembler;
 Options.MCOptions.ShowMCEncoding = ShowMCEncoding;
 Options.MCOptions.MCUseDwarfDirectory = EnableDwarfDirectory;
Index: llvm/test/CodeGen/X86/explicit-section-mergeable.ll
===
--- llvm/test/CodeGen/X86/explicit-section-mergeable.ll
+++ llvm/test/CodeGen/X86/explicit-section-mergeable.ll
@@ -282,15 +282,21 @@
 ;; --no-integrated-as avoids the use of ",unique," for compatibility with older binutils.
 
 ;; Error if an incompatible symbol is explicitly placed into a mergeable section.
-; RUN: not llc < %s -mtriple=x86_64 --no-integrated-as 2>&1 \
+; RUN: not llc < %s -mtriple=x86_64 --no-integrated-as -binutils-version=2.34 2>&1 \
 ; RUN: | FileCheck %s --check-prefix=NO-I-AS-ERR
 ; NO-I-AS-ERR: error: Symbol 'explicit_default_1' from module '' required a section with entry-size=0 but was placed in section '.rodata.cst16' with entry-size=16: Explicit assignment by pragma or attribute of an incompatible symbol to this section?
 ; NO-I-AS-ERR: error: Symbol 'explicit_default_4' from module '' required a section with entry-size=0 but was placed in section '.debug_str' with entry-size=1: Explicit assignment by pragma or attribute of an incompatible symbol to this section?
 ; NO-I-AS-ERR: error: Symbol 'explicit_implicit_2' from module '' required a section with entry-size=0 but was placed in section '.rodata.str1.1' with entry-size=1: Explicit assignment by pragma or attribute of an incompatible symbol to this section?
 ; NO-I-AS-ERR: error: Symbol 'explicit_implicit_4' from module '' required a section with entry-size=0 but was placed in section '.rodata.str1.1' with entry-size=1: Explicit assignment by pragma or attribute of an incompatible symbol to this section?
 
+;; For GNU as before 2.35,
 ;; Don't create mergeable sections for globals with an explicit section name.
 ; RUN: echo '@explicit = unnamed_addr constant [2 x i16] [i16 1, i16 1], section ".explicit"' > %t.no_i_as.ll
-; RUN: llc < %t.no_i_as.ll -mtriple=x86_64 --no-integrated-as 2>&1 \
-; RUN: | FileCheck %s --check-prefix=NO-I-AS
-; NO-I-AS: .section .explicit,"a",@progbits
+; RUN: llc < %t.no_i_as.ll -mtriple=x86_64 --no-integrated-as -binutils-version=2.34 2>&1 \
+; RUN: | FileCheck %s --check-prefix=NO-I-AS-OLD
+; NO-I-AS-OLD: .section .explicit,"a",@progbits
+; RUN: llc < %t.no_i_as.ll -mtriple=x86_64 --no-integrated-as -binutils-version=2.35 2>&1 \
+; RUN: | FileCheck %s --check-prefix=NO-I-AS-NEW
+; RUN: llc < %t.no_i_as.ll -mtriple=x86_64 --no-integrated-as -binutils-version=none 2>&1 \
+; RUN: | FileCheck %s --check-prefix=NO-I-AS-NEW
+; NO-I-AS-NEW: .section .explicit,"aM",@progbits,4,unique,1
Index: llvm/lib/Target/TargetMachine.cpp

[PATCH] D94583: [RISCV] Update V extension to v1.0-draft 08a0b464.

[Evandro Menezes via Phabricator via cfe-commits]

evandro added a comment.

Also, when the V spec becomes official, it'll be labeled v2.0.  Therefore, as 
long as v0.9 or v1.0 is implemented, V is only available as an experimental 
feature.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94583

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

[PATCH] D94745: [OpenMP][WIP] Build the deviceRTLs with OpenMP instead of target dependent language - NOT FOR REVIEW

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield added inline comments.



Comment at: openmp/libomptarget/deviceRTLs/common/omptarget.h:301
+extern DEVICE uint8_t parallelLevel[MAX_THREADS_PER_TEAM / WARPSIZE];
+#pragma omp allocate(parallelLevel) allocator(omp_pteam_mem_alloc)
+extern DEVICE uint16_t EXTERN_SHARED(threadLimit);

This will break on amdgpu, at least until the cmake for amdgpu changes over to 
openmp

If we spell it like:
```
#if _OPENMP
extern DEVICE uint8_t parallelLevel[MAX_THREADS_PER_TEAM / WARPSIZE];
#pragma omp allocate(parallelLevel) allocator(omp_pteam_mem_alloc)
#else
extern DEVICE
uint8_t EXTERN_SHARED(parallelLevel)[MAX_THREADS_PER_TEAM / WARPSIZE];
#endif
```

then amdgpu will continue working. Iirc this is the only shared array variable, 
the rest will be fine.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94745

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

[PATCH] D92983: SourceManager: Migrate to FileEntryRef in getOrCreateContentCache, NFC

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

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG8d67b9e2461d: SourceManager: Migrate to FileEntryRef in 
getOrCreateContentCache, NFC (authored by dexonsmith).

Changed prior to commit:
  https://reviews.llvm.org/D92983?vs=310717=319171#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92983

Files:
  clang/include/clang/Basic/SourceManager.h
  clang/lib/Basic/SourceManager.cpp
  clang/lib/Serialization/ASTReader.cpp

Index: clang/lib/Serialization/ASTReader.cpp
===
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -1499,7 +1499,7 @@
 // we will also try to fail gracefully by setting up the SLocEntry.
 unsigned InputID = Record[4];
 InputFile IF = getInputFile(*F, InputID);
-const FileEntry *File = IF.getFile();
+Optional File = IF.getFile();
 bool OverriddenBuffer = IF.isOverridden();
 
 // Note that we only check if a File was returned. If it was out-of-date
@@ -1515,9 +1515,8 @@
 }
 SrcMgr::CharacteristicKind
   FileCharacter = (SrcMgr::CharacteristicKind)Record[2];
-// FIXME: The FileID should be created from the FileEntryRef.
-FileID FID = SourceMgr.createFileID(File, IncludeLoc, FileCharacter,
-ID, BaseOffset + Record[0]);
+FileID FID = SourceMgr.createFileID(*File, IncludeLoc, FileCharacter, ID,
+BaseOffset + Record[0]);
 SrcMgr::FileInfo  =
   const_cast(SourceMgr.getSLocEntry(FID).getFile());
 FileInfo.NumCreatedFIDs = Record[5];
@@ -1533,14 +1532,14 @@
 }
 
 const SrcMgr::ContentCache  =
-SourceMgr.getOrCreateContentCache(File, isSystem(FileCharacter));
+SourceMgr.getOrCreateContentCache(*File, isSystem(FileCharacter));
 if (OverriddenBuffer && !ContentCache.BufferOverridden &&
 ContentCache.ContentsEntry == ContentCache.OrigEntry &&
 !ContentCache.getBufferIfLoaded()) {
   auto Buffer = ReadBuffer(SLocEntryCursor, File->getName());
   if (!Buffer)
 return true;
-  SourceMgr.overrideFileContents(File, std::move(Buffer));
+  SourceMgr.overrideFileContents(*File, std::move(Buffer));
 }
 
 break;
Index: clang/lib/Basic/SourceManager.cpp
===
--- clang/lib/Basic/SourceManager.cpp
+++ clang/lib/Basic/SourceManager.cpp
@@ -385,10 +385,8 @@
   }
 }
 
-ContentCache ::getOrCreateContentCache(const FileEntry *FileEnt,
+ContentCache ::getOrCreateContentCache(FileEntryRef FileEnt,
  bool isSystemFile) {
-  assert(FileEnt && "Didn't specify a file entry to use?");
-
   // Do we already have information about this file?
   ContentCache * = FileInfos[FileEnt];
   if (Entry)
@@ -414,7 +412,7 @@
 
   Entry->IsFileVolatile = UserFilesAreVolatile && !isSystemFile;
   Entry->IsTransient = FilesAreTransient;
-  Entry->BufferOverridden |= FileEnt->isNamedPipe();
+  Entry->BufferOverridden |= FileEnt.isNamedPipe();
 
   return *Entry;
 }
@@ -542,7 +540,7 @@
SourceLocation IncludePos,
SrcMgr::CharacteristicKind FileCharacter,
int LoadedID, unsigned LoadedOffset) {
-  SrcMgr::ContentCache  = getOrCreateContentCache((),
+  SrcMgr::ContentCache  = getOrCreateContentCache(SourceFile,
  isSystem(FileCharacter));
 
   // If this is a named pipe, immediately load the buffer to ensure subsequent
@@ -682,13 +680,13 @@
 
 llvm::Optional
 SourceManager::getMemoryBufferForFileOrNone(const FileEntry *File) {
-  SrcMgr::ContentCache  = getOrCreateContentCache(File);
+  SrcMgr::ContentCache  = getOrCreateContentCache(File->getLastRef());
   return IR.getBufferOrNone(Diag, getFileManager(), SourceLocation());
 }
 
 void SourceManager::overrideFileContents(
 const FileEntry *SourceFile, std::unique_ptr Buffer) {
-  SrcMgr::ContentCache  = getOrCreateContentCache(SourceFile);
+  SrcMgr::ContentCache  = getOrCreateContentCache(SourceFile->getLastRef());
 
   IR.setBuffer(std::move(Buffer));
   IR.BufferOverridden = true;
@@ -716,12 +714,12 @@
   if (!BypassFile)
 return None;
 
-  (void)getOrCreateContentCache(>getFileEntry());
+  (void)getOrCreateContentCache(*BypassFile);
   return BypassFile;
 }
 
 void SourceManager::setFileIsTransient(const FileEntry *File) {
-  getOrCreateContentCache(File).IsTransient = true;
+  getOrCreateContentCache(File->getLastRef()).IsTransient = true;
 }
 
 Optional
Index: clang/include/clang/Basic/SourceManager.h
===
--- clang/include/clang/Basic/SourceManager.h

[PATCH] D94500: Rework Whitesmiths mode to use line-level values in UnwrappedLineParser

[Tim Wojtulewicz via Phabricator via cfe-commits]

timwoj updated this revision to Diff 319170.
timwoj added a comment.

Restored lost single-nesting namespace test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94500

Files:
  clang/lib/Format/UnwrappedLineFormatter.cpp
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/lib/Format/UnwrappedLineParser.h
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -13543,6 +13543,7 @@
WhitesmithsBraceStyle);
   */
 
+  WhitesmithsBraceStyle.NamespaceIndentation = FormatStyle::NI_None;
   verifyFormat("namespace a\n"
"  {\n"
"class A\n"
@@ -13567,6 +13568,89 @@
"  } // namespace a",
WhitesmithsBraceStyle);
 
+  verifyFormat("namespace a\n"
+   "  {\n"
+   "namespace b\n"
+   "  {\n"
+   "class A\n"
+   "  {\n"
+   "  void f()\n"
+   "{\n"
+   "if (true)\n"
+   "  {\n"
+   "  a();\n"
+   "  b();\n"
+   "  }\n"
+   "}\n"
+   "  void g()\n"
+   "{\n"
+   "return;\n"
+   "}\n"
+   "  };\n"
+   "struct B\n"
+   "  {\n"
+   "  int x;\n"
+   "  };\n"
+   "  } // namespace b\n"
+   "  } // namespace a",
+   WhitesmithsBraceStyle);
+
+  WhitesmithsBraceStyle.NamespaceIndentation = FormatStyle::NI_Inner;
+  verifyFormat("namespace a\n"
+   "  {\n"
+   "namespace b\n"
+   "  {\n"
+   "  class A\n"
+   "{\n"
+   "void f()\n"
+   "  {\n"
+   "  if (true)\n"
+   "{\n"
+   "a();\n"
+   "b();\n"
+   "}\n"
+   "  }\n"
+   "void g()\n"
+   "  {\n"
+   "  return;\n"
+   "  }\n"
+   "};\n"
+   "  struct B\n"
+   "{\n"
+   "int x;\n"
+   "};\n"
+   "  } // namespace b\n"
+   "  } // namespace a",
+   WhitesmithsBraceStyle);
+
+  WhitesmithsBraceStyle.NamespaceIndentation = FormatStyle::NI_All;
+  verifyFormat("namespace a\n"
+   "  {\n"
+   "  namespace b\n"
+   "{\n"
+   "class A\n"
+   "  {\n"
+   "  void f()\n"
+   "{\n"
+   "if (true)\n"
+   "  {\n"
+   "  a();\n"
+   "  b();\n"
+   "  }\n"
+   "}\n"
+   "  void g()\n"
+   "{\n"
+   "return;\n"
+   "}\n"
+   "  };\n"
+   "struct B\n"
+   "  {\n"
+   "  int x;\n"
+   "  };\n"
+   "} // namespace b\n"
+   "  }   // namespace a",
+   WhitesmithsBraceStyle);
+
   verifyFormat("void f()\n"
"  {\n"
"  if (true)\n"
@@ -13601,7 +13685,7 @@
"  }\n",
WhitesmithsBraceStyle);
 
-  WhitesmithsBraceStyle.IndentCaseBlocks = true;
+  WhitesmithsBraceStyle.IndentCaseLabels = true;
   verifyFormat("void switchTest1(int a)\n"
"  {\n"
"  switch (a)\n"
@@ -13609,7 +13693,7 @@
"case 2:\n"
"  {\n"
"  }\n"
-   "break;\n"
+   "  break;\n"
"}\n"
"  }\n",
WhitesmithsBraceStyle);
@@ -13619,7 +13703,7 @@
"  switch (a)\n"
"{\n"
"case 0:\n"
-   "break;\n"
+   "  break;\n"
"case 1:\n"
"  {\n"
"  break;\n"
@@ -13627,9 +13711,9 @@
"case 2:\n"
"  {\n"
"  }\n"
-   "break;\n"
+   "  break;\n"
"default:\n"
-   "break;\n"
+   "  break;\n"
"}\n"
"  }\n",
WhitesmithsBraceStyle);
@@ -13642,17 +13726,17 @@
"  {\n"
"  foo(x);\n"
"  }\n"
-   "break;\n"
+   "  break;\n"
"

[PATCH] D85474: Add -fbinutils-version= to gate ELF features on the specified binutils version

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added inline comments.



Comment at: clang/docs/ReleaseNotes.rst:103-104
+  For example, ``-fbinutils-version=2.35`` means compatibility with GNU as/ld
+  before 2.35 is not needed: new features can be used and don't work around old
+  GNU as/ld bugs.
 

(Just minor copy-editing.)



Comment at: clang/include/clang/Basic/CodeGenOptions.h:129-131
+  // If set, override the default value of MCAsmInfo::BinutilsVersion. "none"
+  // means that features not implemented by any known release can be used. If
+  // DisableIntegratedAS is specified, this also affects assembly output.

I don't understand the description of "none" here; is there a spurious "not" in 
this sentence? Does "none" mean "features implemented by any known release can 
be used" (eg, I promise my binutils is not older than my LLVM), or "features 
implemented by every known release can be used" (eg, my binutils might be 
arbitrarily old, try to be compatible with it)?

I think perhaps this would express what you mean: " "none" means that all ELF 
features can be used, regardless of binutils support."

(Please also update the documentation in Options.td to match any changes here.)



Comment at: clang/include/clang/Driver/Options.td:3449
+  HelpText<"Produced object files can use ELF features only supported by ld 
newer than the specified version. If"
+  "-fno-integrated-as is specified, produced assembly can use newer ELF 
features. "
+  "'none' means that features not implemented by any known release can be 
used">;

The suggestion I take from this is that if `-fintegrated-as` is used, produced 
assembly cannot use newer ELF features, but I don't think that's right -- I'd 
expect that under `-fintegrated-as`, we get to use new ELF features so long as 
we think the linker supports them, regardless of what we think `gas` supports.

Also, "newer ELF features" is not clear to me: newer than what? (Newer than the 
specified version?)



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:4772
+StringRef V = A->getValue();
+int Num;
+if (V == "none")

Perhaps use an unsigned type, so that `-fbinutils-version=3.-14` is properly 
rejected.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:4642
+int Num;
+if (V == "future")
+  A->render(Args, CmdArgs);

dblaikie wrote:
> ro wrote:
> > MaskRay wrote:
> > > phosek wrote:
> > > > Another option would be `unstable`.
> > > I picked `future` because there is an precedent: `-mcpu=future` is used 
> > > by some ppc folks.
> > I fear that's a terrible precedent: this name had to be chosen because for 
> > some unknown, but certainly silly, reason, IBM didn't wan't to call it 
> > `power10` before release.
> A little more respect for other developers would be great here - hard to know 
> it's silly if you don't know the reason & even if you think it is silly, 
> folks may not have the same goals/requirements you do.
> 
> (that's not to dismiss the concern that the precedent for naming -mcpu may 
> not apply as well here (not suggesting it doesn't either - I have no opinion 
> either way))
How about `latest`? (As in, "I promise I have at least the latest version of 
binutils that you've heard of")



Comment at: llvm/include/llvm/MC/MCAsmInfo.h:407
+  // Generated object files can only use features supported by GNU ld of this
+  // binutils version or later.
+  std::pair BinutilsVersion = {2, 26};

(Support by only a later version isn't enough.)



Comment at: llvm/lib/Target/TargetMachine.cpp:287
+  if (Version == "none")
+return {INT_MAX, 0}; // Make binutilsIsAtLeast() fail.
+  std::pair Ret;




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85474

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

[PATCH] D95409: [clang] implicitly delete space ship operator with function pointers

[Matheus Izvekov via Phabricator via cfe-commits]

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

See bug #48856

Definitions of classes with member function pointers and default
spaceship operator were getting accepted with no diagnostic on
release build, and triggering assert on builds with runtime checks
enabled. Diagnostics were only produced when actually comparing
instances of such classes.

This patch makes it so Spaceship and Less operators are not considered
as builtin operator candidates for function pointers, producing
equivalent diagnostics for the cases where pointers to member function
and pointers to data members are used instead.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D95409

Files:
  clang/lib/Sema/SemaDeclCXX.cpp


Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -7677,7 +7677,8 @@
 
 if (Args[0]->getType()->isOverloadableType())
   S.LookupOverloadedBinOp(CandidateSet, OO, Fns, Args);
-else {
+else if (OO == OO_EqualEqual ||
+ !Args[0]->getType()->isFunctionPointerType()) {
   // FIXME: We determine whether this is a valid expression by checking to
   // see if there's a viable builtin operator candidate for it. That isn't
   // really what the rules ask us to do, but should give the right results.


Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -7677,7 +7677,8 @@
 
 if (Args[0]->getType()->isOverloadableType())
   S.LookupOverloadedBinOp(CandidateSet, OO, Fns, Args);
-else {
+else if (OO == OO_EqualEqual ||
+ !Args[0]->getType()->isFunctionPointerType()) {
   // FIXME: We determine whether this is a valid expression by checking to
   // see if there's a viable builtin operator candidate for it. That isn't
   // really what the rules ask us to do, but should give the right results.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D95408: [Sema][C] members of anonymous struct inherit QualType

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added inline comments.



Comment at: clang/lib/Sema/SemaDecl.cpp:5179
   if (RecordDecl *OwningClass = dyn_cast(Owner)) {
-Anon = FieldDecl::Create(
-Context, OwningClass, DS.getBeginLoc(), Record->getLocation(),
-/*IdentifierInfo=*/nullptr, Context.getTypeDeclType(Record), TInfo,
-/*BitWidth=*/nullptr, /*Mutable=*/false,
-/*InitStyle=*/ICIS_NoInit);
+QualType QT = Context.getTypeDeclType(Record);
+if (!getLangOpts().CPlusPlus) {

Maybe there's a way to have `Context.getTypeDeclType` return the qualified type 
here? (I couldn't find a way, but only looked today).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95408

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

[PATCH] D94745: [OpenMP][WIP] Build the deviceRTLs with OpenMP instead of target dependent language - NOT FOR REVIEW

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield added inline comments.



Comment at: openmp/libomptarget/deviceRTLs/nvptx/src/target_impl.h:98
 #ifndef CUDA_VERSION
 #error CUDA_VERSION macro is undefined, something wrong with cuda.
 #endif

This should be firing now that cuda.h is removed


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94745

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

[PATCH] D95408: [Sema][C] members of anonymous struct inherit QualType

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers created this revision.
nickdesaulniers added reviewers: rsmith, aaron.ballman.
nickdesaulniers requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

When declaring an anonymous struct with `const` or `volatile`
qualifiers, ensure the members of that anonymous struct are created with
that qualifier.

GCC behaves this way, and WG14 is currently discussing this behavior.

See also:

- https://bugs.llvm.org/show_bug.cgi?id=48755
- https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98826


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D95408

Files:
  clang/lib/Sema/SemaDecl.cpp
  clang/test/AST/ast-dump-decl.c
  clang/test/AST/ast-dump-records.c
  clang/test/Sema/struct-decl-anonymous-members.c
  clang/test/Sema/struct-decl.c

Index: clang/test/Sema/struct-decl.c
===
--- clang/test/Sema/struct-decl.c
+++ clang/test/Sema/struct-decl.c
@@ -110,3 +110,8 @@
   struct FlexibleArrayMem a; // expected-warning {{field 'a' with variable sized type 'struct FlexibleArrayMem' not at the end of a struct or class is a GNU extension}}
   struct {};
 };
+
+struct QualifiedAnonymousMembers {
+  const struct { int foo; };
+  volatile struct { int bar; };
+};
Index: clang/test/Sema/struct-decl-anonymous-members.c
===
--- /dev/null
+++ clang/test/Sema/struct-decl-anonymous-members.c
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 %s -fsyntax-only -verify
+struct dummy {
+  const struct { // expected-note {{non-static data member '' declared const here}}
+int a;
+// 'b' here should be 'const volatile' qualified.
+volatile struct { int b; };
+  };
+  int c;
+};
+
+extern void fn(const int *); // expected-note {{passing argument to parameter here}}
+extern void fn2(volatile int *); // expected-note {{passing argument to parameter here}}
+
+void test(struct dummy *a) {
+  fn(>b); // expected-warning {{passing 'const volatile int *' to parameter of type 'const int *' discards qualifiers}}
+  fn2(>b); // expected-warning {{passing 'const volatile int *' to parameter of type 'volatile int *' discards qualifiers}}
+  a->a = a->c; // expected-error {{cannot assign to non-static data member '' with const-qualified type}}
+}
Index: clang/test/AST/ast-dump-records.c
===
--- clang/test/AST/ast-dump-records.c
+++ clang/test/AST/ast-dump-records.c
@@ -156,3 +156,24 @@
   // CHECK-NEXT: Field 0x{{[^ ]*}} 'f' 'int'
 };
 
+struct H {
+  const struct {
+int bar;
+  };
+  volatile struct {
+int baz;
+  };
+};
+// CHECK: RecordDecl 0x{{[^ ]*}}  line:[[@LINE-8]]:8 struct H definition
+// CHECK-NEXT: RecordDecl 0x{{[^ ]*}}  line:[[@LINE-8]]:9 struct definition
+// CHECK-NEXT: FieldDecl 0x{{[^ ]*}}  col:9 bar 'int'
+// CHECK-NEXT: FieldDecl 0x{{[^ ]*}}  col:9 implicit 'const struct H::(anonymous at {{.*}}:[[@LINE-10]]:9)'
+// CHECK-NEXT: IndirectFieldDecl 0x{{[^ ]*}}  col:9 implicit bar 'int'
+// CHECK-NEXT: Field 0x{{[^ ]*}} '' 'const struct H::(anonymous at {{.*}}:[[@LINE-12]]:9)'
+// CHECK-NEXT: Field 0x{{[^ ]*}} 'bar' 'int'
+// CHECK-NEXT: RecordDecl 0x{{[^ ]*}}  line:[[@LINE-11]]:12 struct definition
+// CHECK-NEXT: FieldDecl 0x{{[^ ]*}}  col:9 baz 'int'
+// CHECK-NEXT: FieldDecl 0x{{[^ ]*}}  col:12 implicit 'volatile struct H::(anonymous at {{.*}}:[[@LINE-13]]:12)'
+// CHECK-NEXT: IndirectFieldDecl 0x{{[^ ]*}}  col:9 implicit baz 'int'
+// CHECK-NEXT: Field 0x{{[^ ]*}} '' 'volatile struct H::(anonymous at {{.*}}:[[@LINE-15]]:12)'
+// CHECK-NEXT: Field 0x{{[^ ]*}} 'baz' 'int'
Index: clang/test/AST/ast-dump-decl.c
===
--- clang/test/AST/ast-dump-decl.c
+++ clang/test/AST/ast-dump-decl.c
@@ -118,11 +118,24 @@
   struct {
 int TestIndirectFieldDecl;
   };
+  const struct {
+int Test48755Const;
+  };
+  volatile struct {
+int Test48755Volatile;
+  };
 };
 // CHECK:  IndirectFieldDecl{{.*}} TestIndirectFieldDecl 'int'
 // CHECK-NEXT:   Field{{.*}} ''
 // CHECK-NEXT:   Field{{.*}} 'TestIndirectFieldDecl'
 
+// CHECK: IndirectFieldDecl{{.*}} Test48755Const 'int'
+// CHECK-NEXT: Field{{.*}} 'const struct testIndirectFieldDecl::{{.*}}'
+
+// CHECK: IndirectFieldDecl{{.*}} Test48755Volatile 'int'
+// CHECK-NEXT: Field{{.*}} 'volatile struct testIndirectFieldDecl::{{.*}}'
+
+
 // FIXME: It would be nice to dump the enum and its enumerators.
 int TestFunctionDecl(int x, enum { e } y) {
   return x;
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -5176,11 +5176,18 @@
   // Create a declaration for this anonymous struct/union.
   NamedDecl *Anon = nullptr;
   if (RecordDecl *OwningClass = dyn_cast(Owner)) {
-Anon = FieldDecl::Create(
-Context, OwningClass, DS.getBeginLoc(), 

[PATCH] D95407: [clangd] NFC Silence some buildbot warnings

[Nathan James via Phabricator via cfe-commits]

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

clang-tools-extra/clangd/unittests/FindTargetTests.cpp:343:29: warning: ISO 
C++11 requires at least one argument for the "..." in a variadic macro
clang-tools-extra/clangd/unittests/FindTargetTests.cpp:947:35: warning: ISO 
C++11 requires at least one argument for the "..." in a variadic macro
clang-tools-extra/clangd/unittests/SelectionTests.cpp:646:17: warning: suggest 
explicit braces to avoid ambiguous 'else' [-Wdangling-else]


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D95407

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


Index: clang-tools-extra/clangd/unittests/SelectionTests.cpp
===
--- clang-tools-extra/clangd/unittests/SelectionTests.cpp
+++ clang-tools-extra/clangd/unittests/SelectionTests.cpp
@@ -641,10 +641,11 @@
   AST.getASTContext(), AST.getTokens(), Test.point("ambiguous"),
   Test.point("ambiguous"), [&](SelectionTree T) {
 // Expect to see the right-biased tree first.
-if (Seen == 0)
+if (Seen == 0) {
   EXPECT_EQ("BinaryOperator", nodeKind(T.commonAncestor()));
-else if (Seen == 1)
+} else if (Seen == 1) {
   EXPECT_EQ("IntegerLiteral", nodeKind(T.commonAncestor()));
+}
 ++Seen;
 return false;
   });
Index: clang-tools-extra/clangd/unittests/FindTargetTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -101,6 +101,9 @@
   << Code
 using ExpectedDecls = std::vector;
 
+#define EXPECT_NO_DECLS(NodeType)  
\
+  EXPECT_THAT(assertNodeAndPrintDecls(NodeType), ::testing::IsEmpty()) << Code
+
 TEST_F(TargetDeclTest, Exprs) {
   Code = R"cpp(
 int f();
@@ -340,7 +343,7 @@
 [[auto]] X = S{};
   )cpp";
   // FIXME: deduced type missing in AST. https://llvm.org/PR42914
-  EXPECT_DECLS("AutoTypeLoc");
+  EXPECT_NO_DECLS("AutoTypeLoc");
 
   Code = R"cpp(
 template 
@@ -944,7 +947,7 @@
 void test(C<[[Foo]]> *p);
   )cpp";
   // FIXME: there's no AST node corresponding to 'Foo', so we're stuck.
-  EXPECT_DECLS("ObjCObjectTypeLoc");
+  EXPECT_NO_DECLS("ObjCObjectTypeLoc");
 }
 
 class FindExplicitReferencesTest : public ::testing::Test {


Index: clang-tools-extra/clangd/unittests/SelectionTests.cpp
===
--- clang-tools-extra/clangd/unittests/SelectionTests.cpp
+++ clang-tools-extra/clangd/unittests/SelectionTests.cpp
@@ -641,10 +641,11 @@
   AST.getASTContext(), AST.getTokens(), Test.point("ambiguous"),
   Test.point("ambiguous"), [&](SelectionTree T) {
 // Expect to see the right-biased tree first.
-if (Seen == 0)
+if (Seen == 0) {
   EXPECT_EQ("BinaryOperator", nodeKind(T.commonAncestor()));
-else if (Seen == 1)
+} else if (Seen == 1) {
   EXPECT_EQ("IntegerLiteral", nodeKind(T.commonAncestor()));
+}
 ++Seen;
 return false;
   });
Index: clang-tools-extra/clangd/unittests/FindTargetTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -101,6 +101,9 @@
   << Code
 using ExpectedDecls = std::vector;
 
+#define EXPECT_NO_DECLS(NodeType)  \
+  EXPECT_THAT(assertNodeAndPrintDecls(NodeType), ::testing::IsEmpty()) << Code
+
 TEST_F(TargetDeclTest, Exprs) {
   Code = R"cpp(
 int f();
@@ -340,7 +343,7 @@
 [[auto]] X = S{};
   )cpp";
   // FIXME: deduced type missing in AST. https://llvm.org/PR42914
-  EXPECT_DECLS("AutoTypeLoc");
+  EXPECT_NO_DECLS("AutoTypeLoc");
 
   Code = R"cpp(
 template 
@@ -944,7 +947,7 @@
 void test(C<[[Foo]]> *p);
   )cpp";
   // FIXME: there's no AST node corresponding to 'Foo', so we're stuck.
-  EXPECT_DECLS("ObjCObjectTypeLoc");
+  EXPECT_NO_DECLS("ObjCObjectTypeLoc");
 }
 
 class FindExplicitReferencesTest : public ::testing::Test {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D94745: [OpenMP][WIP] Build the deviceRTLs with OpenMP instead of target dependent language - NOT FOR REVIEW

[Shilei Tian via Phabricator via cfe-commits]

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

Final refinement before moving to review


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94745

Files:
  clang/lib/Driver/ToolChains/Cuda.cpp
  clang/test/Driver/Inputs/libomptarget/libomptarget-nvptx-cuda_80-sm_20.bc
  clang/test/Driver/Inputs/libomptarget/libomptarget-nvptx-sm_20.bc
  clang/test/Driver/openmp-offload-gpu.c
  openmp/libomptarget/deviceRTLs/common/allocator.h
  openmp/libomptarget/deviceRTLs/common/omptarget.h
  openmp/libomptarget/deviceRTLs/common/src/omp_data.cu
  openmp/libomptarget/deviceRTLs/common/src/reduction.cu
  openmp/libomptarget/deviceRTLs/nvptx/CMakeLists.txt
  openmp/libomptarget/deviceRTLs/nvptx/src/nvptx_interface.h
  openmp/libomptarget/deviceRTLs/nvptx/src/target_impl.cu
  openmp/libomptarget/deviceRTLs/nvptx/src/target_impl.h

Index: openmp/libomptarget/deviceRTLs/nvptx/src/target_impl.h
===
--- openmp/libomptarget/deviceRTLs/nvptx/src/target_impl.h
+++ openmp/libomptarget/deviceRTLs/nvptx/src/target_impl.h
@@ -13,18 +13,16 @@
 #define _TARGET_IMPL_H_
 
 #include 
-#include 
 #include 
 #include 
 #include 
 
 #include "nvptx_interface.h"
 
-#define DEVICE __device__
-#define INLINE __forceinline__ DEVICE
-#define NOINLINE __noinline__ DEVICE
-#define SHARED __shared__
-#define ALIGN(N) __align__(N)
+#define DEVICE
+#define INLINE inline __attribute__((always_inline))
+#define NOINLINE __attribute__((noinline))
+#define ALIGN(N) __attribute__((aligned(N)))
 
 
 // Kernel options
Index: openmp/libomptarget/deviceRTLs/nvptx/src/target_impl.cu
===
--- openmp/libomptarget/deviceRTLs/nvptx/src/target_impl.cu
+++ openmp/libomptarget/deviceRTLs/nvptx/src/target_impl.cu
@@ -14,8 +14,6 @@
 #include "target_impl.h"
 #include "common/debug.h"
 
-#include 
-
 DEVICE void __kmpc_impl_unpack(uint64_t val, uint32_t , uint32_t ) {
   asm volatile("mov.b64 {%0,%1}, %2;" : "=r"(lo), "=r"(hi) : "l"(val));
 }
Index: openmp/libomptarget/deviceRTLs/nvptx/src/nvptx_interface.h
===
--- openmp/libomptarget/deviceRTLs/nvptx/src/nvptx_interface.h
+++ openmp/libomptarget/deviceRTLs/nvptx/src/nvptx_interface.h
@@ -11,7 +11,8 @@
 
 #include 
 
-#define EXTERN extern "C" __device__
+#define EXTERN extern "C"
+
 typedef uint32_t __kmpc_impl_lanemask_t;
 typedef uint32_t omp_lock_t; /* arbitrary type of the right length */
 
Index: openmp/libomptarget/deviceRTLs/nvptx/CMakeLists.txt
===
--- openmp/libomptarget/deviceRTLs/nvptx/CMakeLists.txt
+++ openmp/libomptarget/deviceRTLs/nvptx/CMakeLists.txt
@@ -10,6 +10,21 @@
 #
 ##===--===##
 
+# TODO: This part needs to be refined when libomptarget is going to support
+# Windows!
+# TODO: This part can also be removed if we can change the clang driver to make
+# it support device only compilation.
+if(CMAKE_HOST_SYSTEM_PROCESSOR MATCHES "x86_64")
+  set(aux_triple x86_64-unknown-linux-gnu)
+elseif(CMAKE_HOST_SYSTEM_PROCESSOR MATCHES "ppc64le")
+  set(aux_triple powerpc64le-unknown-linux-gnu)
+elseif(CMAKE_HOST_SYSTEM_PROCESSOR MATCHES "aarch64")
+  set(aux_triple aarch64-unknown-linux-gnu)
+else()
+  libomptarget_say("Not building CUDA offloading device RTL: unknown host arch: ${CMAKE_HOST_SYSTEM_PROCESSOR}")
+  return()
+endif()
+
 get_filename_component(devicertl_base_directory
   ${CMAKE_CURRENT_SOURCE_DIR}
   DIRECTORY)
@@ -79,61 +94,82 @@
 )
 
 # Set flags for LLVM Bitcode compilation.
-set(bc_flags ${LIBOMPTARGET_NVPTX_SELECTED_CUDA_COMPILER_FLAGS}
+set(bc_flags -S -x c++
+ -target nvptx64
+ -Xclang -emit-llvm-bc
+ -Xclang -aux-triple -Xclang ${aux_triple}
+ -fopenmp -fopenmp-cuda-mode -Xclang -fopenmp-is-device
+ -D__CUDACC__
  -I${devicertl_base_directory}
  -I${devicertl_nvptx_directory}/src)
 
 if(${LIBOMPTARGET_NVPTX_DEBUG})
-  set(bc_flags ${bc_flags} -DOMPTARGET_NVPTX_DEBUG=-1)
+  list(APPEND bc_flags -DOMPTARGET_NVPTX_DEBUG=-1)
 else()
-  set(bc_flags ${bc_flags} -DOMPTARGET_NVPTX_DEBUG=0)
+  list(APPEND bc_flags -DOMPTARGET_NVPTX_DEBUG=0)
 endif()
 
 # Create target to build all Bitcode libraries.
 add_custom_target(omptarget-nvptx-bc)
 
-# Generate a Bitcode library for all the compute capabilities the user requested.
+# This map is from clang/lib/Driver/ToolChains/Cuda.cpp.
+# The last element is the default case.
+set(cuda_version_list 110 102 101 100 92 91 90 80)
+set(ptx_feature_list 70 65 64 63 

[PATCH] D95249: [NFC] Disallow unused prefixes in clang/test/Analysis

[Mircea Trofin via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG91b61abafb5a: [NFC] Disallow unused prefixes in 
clang/test/Analysis (authored by mtrofin).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95249

Files:
  clang/test/Analysis/auto-obj-dtors-cfg-output.cpp
  clang/test/Analysis/cfg-rich-constructors.cpp
  clang/test/Analysis/cfg-rich-constructors.mm
  clang/test/Analysis/cfg.c
  clang/test/Analysis/exploded-graph-rewriter/trimmers.dot
  clang/test/Analysis/lit.local.cfg

Index: clang/test/Analysis/lit.local.cfg
===
--- clang/test/Analysis/lit.local.cfg
+++ clang/test/Analysis/lit.local.cfg
@@ -1,5 +1,5 @@
 # -*- Python -*- vim: set ft=python ts=4 sw=4 expandtab tw=79:
-
+from lit.llvm.subst import ToolSubst
 import site
 
 # Load the custom analyzer test format, which runs the test again with Z3 if it
@@ -26,3 +26,9 @@
 
 if not config.root.clang_staticanalyzer:
 config.unsupported = True
+
+fc = ToolSubst('FileCheck', unresolved='fatal')
+# Insert this first. Then, we'll first update the blank FileCheck command; then,
+# the default substitution of FileCheck will replace it to its full path.
+config.substitutions.insert(0, (fc.regex,
+'FileCheck --allow-unused-prefixes=false'))
Index: clang/test/Analysis/exploded-graph-rewriter/trimmers.dot
===
--- clang/test/Analysis/exploded-graph-rewriter/trimmers.dot
+++ clang/test/Analysis/exploded-graph-rewriter/trimmers.dot
@@ -1,17 +1,17 @@
 // RUN: %exploded_graph_rewriter %s \
 // RUN: | FileCheck %s -check-prefixes=ONE,TWO,THREE,FOUR
 // RUN: %exploded_graph_rewriter -s %s \
-// RUN: | FileCheck %s -check-prefixes=ONE,TWO,NOTHREE,FOUR
+// RUN: | FileCheck %s -check-prefixes=ONE,TWO,NOTTHREE,FOUR
 // RUN: %exploded_graph_rewriter --to=0x2 %s \
-// RUN: | FileCheck %s -check-prefixes=ONE,TWO,NOTHREE,NOFOUR
+// RUN: | FileCheck %s -check-prefixes=ONE,TWO,NOTTHREE,NOTFOUR
 // RUN: %exploded_graph_rewriter --to 2 %s \
-// RUN: | FileCheck %s -check-prefixes=ONE,TWO,NOTHREE,NOFOUR
+// RUN: | FileCheck %s -check-prefixes=ONE,TWO,NOTTHREE,NOTFOUR
 // RUN: %exploded_graph_rewriter --to 2,3 %s \
-// RUN: | FileCheck %s -check-prefixes=ONE,TWO,THREE,NOFOUR
+// RUN: | FileCheck %s -check-prefixes=ONE,TWO,THREE,NOTFOUR
 // RUN: %exploded_graph_rewriter --to 4 %s \
 // RUN: | FileCheck %s -check-prefixes=ONE,TWO,THREE,FOUR
 // RUN: %exploded_graph_rewriter --to 4 -s %s \
-// RUN: | FileCheck %s -check-prefixes=ONE,TWO,NOTHREE,FOUR
+// RUN: | FileCheck %s -check-prefixes=ONE,TWO,NOTTHREE,FOUR
 
 Node0x1 [shape=record,label=
  "{{ "state_id": 0, "program_state": null, "program_points": [
Index: clang/test/Analysis/cfg.c
===
--- clang/test/Analysis/cfg.c
+++ clang/test/Analysis/cfg.c
@@ -1,5 +1,5 @@
 // RUN: %clang_analyze_cc1 -analyzer-checker=debug.DumpCFG -triple x86_64-apple-darwin12 -fheinous-gnu-extensions %s > %t 2>&1
-// RUN: FileCheck --input-file=%t -check-prefixes=CHECK,WARNINGS %s
+// RUN: FileCheck --input-file=%t --check-prefix=CHECK %s
 
 // This file is the C version of cfg.cpp.
 // Tests that are C-specific should go into this file.
Index: clang/test/Analysis/cfg-rich-constructors.mm
===
--- clang/test/Analysis/cfg-rich-constructors.mm
+++ clang/test/Analysis/cfg-rich-constructors.mm
@@ -1,11 +1,11 @@
 // RUN: %clang_analyze_cc1 -analyzer-checker=debug.DumpCFG -triple x86_64-apple-darwin12 -std=c++11 -w %s > %t 2>&1
-// RUN: FileCheck --input-file=%t -check-prefixes=CHECK,CXX11,ELIDE,CXX11-ELIDE %s
+// RUN: FileCheck --input-file=%t -check-prefixes=CHECK,CXX11,CXX11-ELIDE %s
 // RUN: %clang_analyze_cc1 -analyzer-checker=debug.DumpCFG -triple x86_64-apple-darwin12 -std=c++17 -w %s > %t 2>&1
-// RUN: FileCheck --input-file=%t -check-prefixes=CHECK,CXX17,ELIDE,CXX17-ELIDE %s
+// RUN: FileCheck --input-file=%t -check-prefixes=CHECK,CXX17 %s
 // RUN: %clang_analyze_cc1 -analyzer-checker=debug.DumpCFG -triple x86_64-apple-darwin12 -std=c++11 -w -analyzer-config elide-constructors=false %s > %t 2>&1
-// RUN: FileCheck --input-file=%t -check-prefixes=CHECK,CXX11,NOELIDE,CXX11-NOELIDE %s
+// RUN: FileCheck --input-file=%t -check-prefixes=CHECK,CXX11,CXX11-NOELIDE %s
 // RUN: %clang_analyze_cc1 -analyzer-checker=debug.DumpCFG -triple x86_64-apple-darwin12 -std=c++17 -w -analyzer-config elide-constructors=false %s > %t 2>&1
-// RUN: FileCheck --input-file=%t -check-prefixes=CHECK,CXX17,NOELIDE,CXX17-NOELIDE %s
+// RUN: FileCheck --input-file=%t -check-prefixes=CHECK,CXX17 %s
 
 class D {
 public:
Index: clang/test/Analysis/cfg-rich-constructors.cpp
===
--- 

[PATCH] D94745: [OpenMP][WIP] Build the deviceRTLs with OpenMP instead of target dependent language - NOT FOR REVIEW

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield added a comment.

> the access size (8 bytes) exceeds the max lock-free size (0  bytes) 
> [-Watomic-alignment

That's not a useful warning. Every size is greater than 0. I guess nvptx hasn't 
set a value somewhere in clang for max lock free size.

Fortunately we only compile with clang, so let's just pass Wno-atomic-alignment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94745

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

[PATCH] D94745: [OpenMP][WIP] Build the deviceRTLs with OpenMP instead of target dependent language - NOT FOR REVIEW

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 added inline comments.



Comment at: openmp/libomptarget/deviceRTLs/nvptx/src/target_impl.h:25
+#define NOINLINE __attribute__((noinline))
+#define ALIGN(N) __attribute__((aligned(N)))
 

JonChesterfield wrote:
> Suggest we drop the DEVICE annotation and change ALIGN to alignas() or 
> similar, but in a later patch. This one is already quite noisy.
yes, just maintain minimal changes in the code for this patch. We can optimize 
everything afterwards.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94745

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

[PATCH] D94745: [OpenMP][WIP] Build the deviceRTLs with OpenMP instead of target dependent language - NOT FOR REVIEW

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 added inline comments.



Comment at: openmp/libomptarget/deviceRTLs/nvptx/CMakeLists.txt:16
+# TODO: This part can also be removed if we can change the clang driver to make
+# it support device only compilation.
+if(CMAKE_HOST_SYSTEM_PROCESSOR MATCHES "x86_64")

JonChesterfield wrote:
> From @jdoerfert,
> 
> ```-Xclang -fopenmp-is-device  -Xclang -emit-llvm-bc``` 
> should do device-only
If I drop `-fopenmp -Xclang -aux-triple -Xclang ${aux_triple}`, a warning will 
be emitted:
```
/home/shiltian/Documents/vscode/llvm-project/openmp/libomptarget/deviceRTLs/nvptx/src/target_impl.cu:129:10:
 warning: large atomic operation may incur significant performance penalty; the 
access size (4 bytes) exceeds the max lock-free size (0  bytes) 
[-Watomic-alignment]
  return __atomic_fetch_add(Address, Val, __ATOMIC_SEQ_CST);
 ^
/home/shiltian/Documents/vscode/llvm-project/openmp/libomptarget/deviceRTLs/nvptx/src/target_impl.cu:136:10:
 warning: large atomic operation may incur significant performance penalty; the 
access size (4 bytes) exceeds the max lock-free size (0  bytes) 
[-Watomic-alignment]
  return __atomic_fetch_max(Address, Val, __ATOMIC_SEQ_CST);
 ^
/home/shiltian/Documents/vscode/llvm-project/openmp/libomptarget/deviceRTLs/nvptx/src/target_impl.cu:141:3:
 warning: large atomic operation may incur significant performance penalty; the 
access size (4 bytes) exceeds the max lock-free size (0  bytes) 
[-Watomic-alignment]
  __atomic_exchange(Address, , , __ATOMIC_SEQ_CST);
  ^
/home/shiltian/Documents/vscode/llvm-project/openmp/libomptarget/deviceRTLs/nvptx/src/target_impl.cu:147:9:
 warning: large atomic operation may incur significant performance penalty; the 
access size (4 bytes) exceeds the max lock-free size (0  bytes) 
[-Watomic-alignment]
  (void)__atomic_compare_exchange(Address, , , false,
^
/home/shiltian/Documents/vscode/llvm-project/openmp/libomptarget/deviceRTLs/nvptx/src/target_impl.cu:155:3:
 warning: large atomic operation may incur significant performance penalty; the 
access size (8 bytes) exceeds the max lock-free size (0  bytes) 
[-Watomic-alignment]
  __atomic_exchange(Address, , , __ATOMIC_SEQ_CST);
  ^
/home/shiltian/Documents/vscode/llvm-project/openmp/libomptarget/deviceRTLs/nvptx/src/target_impl.cu:161:10:
 warning: large atomic operation may incur significant performance penalty; the 
access size (8 bytes) exceeds the max lock-free size (0  bytes) 
[-Watomic-alignment]
  return __atomic_fetch_add(Address, Val, __ATOMIC_SEQ_CST);
 ^
6 warnings generated.
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94745

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

[PATCH] D94745: [OpenMP][WIP] Build the deviceRTLs with OpenMP instead of target dependent language - NOT FOR REVIEW

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield added a comment.

Couple of minor suggestions inline, but overall this looks pretty good. 
Hopefully device-only compilation works already, the rest could be left for 
after this patch.




Comment at: openmp/libomptarget/deviceRTLs/nvptx/CMakeLists.txt:100
+ -fopenmp -Xclang -fopenmp-is-device
+ -D__CUDACC__
  -I${devicertl_base_directory}

jdoerfert wrote:
> JonChesterfield wrote:
> > tianshilei1992 wrote:
> > > JonChesterfield wrote:
> > > > This is suspect - why does openmp want to claim to be cuda?
> > > To maintain minimal change. There is an include wrapped into a macro in 
> > > `interface.h`. For AMD GPU, it includes one header in AMD implementation, 
> > > and for CUDA device, it includes a header in NVPTX implementation.
> > Ah, that's probably my fault. May as well leave it for now.
> > 
> > I think we should expose a macro for openmp that indicates whether we're 
> > doing offloading to nvptx, or offloading to amdgpu, or just compiling for 
> > the host. Or, I think equivalently, replace some `#if` with variant.
> Please don't use defines if we have `begin/end declare variant` for it.
Variant sounds good. Should also be able to use `#ifdef __CUDA_ARCH__`, as 
amdgpu shouldn't be setting that



Comment at: openmp/libomptarget/deviceRTLs/nvptx/CMakeLists.txt:16
+# TODO: This part can also be removed if we can change the clang driver to make
+# it support device only compilation.
+if(CMAKE_HOST_SYSTEM_PROCESSOR MATCHES "x86_64")

From @jdoerfert,

```-Xclang -fopenmp-is-device  -Xclang -emit-llvm-bc``` 
should do device-only



Comment at: openmp/libomptarget/deviceRTLs/nvptx/CMakeLists.txt:115
 
-# Generate a Bitcode library for all the compute capabilities the user 
requested.
+# This correlation is from clang/lib/Driver/ToolChains/Cuda.cpp.
+# The last element is the default case.

s/correlation/correspondence, or maybe mapping



Comment at: openmp/libomptarget/deviceRTLs/nvptx/src/target_impl.h:25
+#define NOINLINE __attribute__((noinline))
+#define ALIGN(N) __attribute__((aligned(N)))
 

Suggest we drop the DEVICE annotation and change ALIGN to alignas() or similar, 
but in a later patch. This one is already quite noisy.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94745

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

[PATCH] D95253: [clang][Fuchsia] Add relative-vtables + asan multilibs

[Leonard Chan via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGc0e94e997473: [clang][Fuchsia] Add relative-vtables + asan 
multilibs (authored by leonardchan).

Changed prior to commit:
  https://reviews.llvm.org/D95253?vs=318588=319142#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95253

Files:
  clang/cmake/caches/Fuchsia-stage2.cmake
  clang/lib/Driver/ToolChains/Fuchsia.cpp
  
clang/test/Driver/Inputs/basic_fuchsia_tree/lib/x86_64-fuchsia/c++/relative-vtables+asan+noexcept/libc++.so
  
clang/test/Driver/Inputs/basic_fuchsia_tree/lib/x86_64-fuchsia/c++/relative-vtables+asan/libc++.so
  clang/test/Driver/fuchsia.cpp

Index: clang/test/Driver/fuchsia.cpp
===
--- clang/test/Driver/fuchsia.cpp
+++ clang/test/Driver/fuchsia.cpp
@@ -96,10 +96,37 @@
 // RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
 // RUN: -fuse-ld=lld 2>&1\
 // RUN: | FileCheck %s -check-prefixes=CHECK-MULTILIB-X86,CHECK-MULTILIB-RELATIVE-VTABLES-NOEXCEPT-X86
+// RUN: %clangxx %s -### --target=x86_64-fuchsia -fexperimental-relative-c++-abi-vtables -fsanitize=address \
+// RUN: -ccc-install-dir %S/Inputs/basic_fuchsia_tree/bin \
+// RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
+// RUN: -fuse-ld=lld 2>&1\
+// RUN: | FileCheck %s -check-prefixes=CHECK-MULTILIB-X86,CHECK-MULTILIB-RELATIVE-VTABLES-ASAN-X86
+// RUN: %clangxx %s -### --target=x86_64-fuchsia -fexperimental-relative-c++-abi-vtables -fno-exceptions -fsanitize=address \
+// RUN: -ccc-install-dir %S/Inputs/basic_fuchsia_tree/bin \
+// RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
+// RUN: -fuse-ld=lld 2>&1\
+// RUN: | FileCheck %s -check-prefixes=CHECK-MULTILIB-X86,CHECK-MULTILIB-RELATIVE-VTABLES-ASAN-NOEXCEPT-X86
+// RUN: %clangxx %s -### --target=x86_64-fuchsia -fno-experimental-relative-c++-abi-vtables \
+// RUN: -ccc-install-dir %S/Inputs/basic_fuchsia_tree/bin \
+// RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
+// RUN: -fuse-ld=lld 2>&1\
+// RUN: | FileCheck %s -check-prefixes=CHECK-MULTILIB-X86
+// RUN: %clangxx %s -### --target=x86_64-fuchsia -fno-experimental-relative-c++-abi-vtables \
+// RUN: -ccc-install-dir %S/Inputs/basic_fuchsia_tree/bin \
+// RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
+// RUN: -fuse-ld=lld 2>&1\
+// RUN: | FileCheck %s -check-prefixes=CHECK-MULTILIB-X86
+// RUN: %clangxx %s -### --target=x86_64-fuchsia -fno-experimental-relative-c++-abi-vtables -fexperimental-relative-c++-abi-vtables \
+// RUN: -ccc-install-dir %S/Inputs/basic_fuchsia_tree/bin \
+// RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
+// RUN: -fuse-ld=lld 2>&1\
+// RUN: | FileCheck %s -check-prefixes=CHECK-MULTILIB-X86
 // CHECK-MULTILIB-X86: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
 // CHECK-MULTILIB-ASAN-X86: "-L{{.*}}{{/|}}..{{/|}}lib{{/|}}x86_64-fuchsia{{/|}}c++{{/|}}asan"
 // CHECK-MULTILIB-NOEXCEPT-X86: "-L{{.*}}{{/|}}..{{/|}}lib{{/|}}x86_64-fuchsia{{/|}}c++{{/|}}noexcept"
 // CHECK-MULTILIB-ASAN-NOEXCEPT-X86: "-L{{.*}}{{/|}}..{{/|}}lib{{/|}}x86_64-fuchsia{{/|}}c++{{/|}}asan+noexcept"
 // CHECK-MULTILIB-RELATIVE-VTABLES-X86: "-L{{.*}}{{/|}}..{{/|}}lib{{/|}}x86_64-fuchsia{{/|}}c++{{/|}}relative-vtables"
 // CHECK-MULTILIB-RELATIVE-VTABLES-NOEXCEPT-X86: "-L{{.*}}{{/|}}..{{/|}}lib{{/|}}x86_64-fuchsia{{/|}}c++{{/|}}relative-vtables+noexcept"
+// CHECK-MULTILIB-RELATIVE-VTABLES-ASAN-X86: "-L{{.*}}{{/|}}..{{/|}}lib{{/|}}x86_64-fuchsia{{/|}}c++{{/|}}relative-vtables+asan"
+// CHECK-MULTILIB-RELATIVE-VTABLES-ASAN-NOEXCEPT-X86: "-L{{.*}}{{/|}}..{{/|}}lib{{/|}}x86_64-fuchsia{{/|}}c++{{/|}}relative-vtables+asan+noexcept"
 // CHECK-MULTILIB-X86: "-L{{.*}}{{/|}}..{{/|}}lib{{/|}}x86_64-fuchsia{{/|}}c++"
Index: clang/lib/Driver/ToolChains/Fuchsia.cpp
===
--- clang/lib/Driver/ToolChains/Fuchsia.cpp
+++ clang/lib/Driver/ToolChains/Fuchsia.cpp
@@ -211,12 +211,22 @@
   .flag("-fexceptions")
   .flag("+fno-exceptions"));
   // Use the relative vtables ABI.
+  // TODO: Remove these multilibs once relative vtables are enabled by default
+  // for Fuchsia.
   Multilibs.push_back(Multilib("relative-vtables", {}, {}, 4)
   .flag("+fexperimental-relative-c++-abi-vtables"));
   Multilibs.push_back(Multilib("relative-vtables+noexcept", {}, {}, 5)
   .flag("+fexperimental-relative-c++-abi-vtables")
   .flag("-fexceptions")
   .flag("+fno-exceptions"));
+  

[PATCH] D90646: [clang] Add warning when `-include-pch` is passed multiple times

[Keith Smiley via Phabricator via cfe-commits]

keith added a comment.

Fair question! I think this case is a bit different since it's quite subtle. 
The strange thing here is that the header you're intending to provide a pch for 
can still be read successfully, but not getting the benefits of the pch that 
you were expecting without knowing about it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90646

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

[PATCH] D95403: [clang-tidy][analyzer][WIP] Clang-tidy reverse integration into Static Analyzer.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ created this revision.
NoQ added reviewers: vsavchenko, xazax.hun, martong, Szelethus, 
baloghadamsoftware, Charusso, steakhal, gribozavr2, alexfh, aaron.ballman.
Herald added subscribers: dang, ASDenysPetrov, phosek, dkrupp, donat.nagy, 
mikhail.ramalho, a.sidorin, rnkovacs, szepet, mgorny.
Herald added a reviewer: jansvoboda11.
NoQ requested review of this revision.
Herald added a reviewer: jdoerfert.
Herald added a subscriber: sstefan1.

This patch allows building a clang binary that has clang-tidy checks embedded 
into the static analyzer. I discussed these plans briefly in 
https://lists.llvm.org/pipermail/cfe-dev/2020-October/067002.html.

WIP because i haven't handled any build-time settings yet. This new facility 
//will be// under a CMake flag and most likely off by default (unless everybody 
suddenly loves this option, but even then, I still have to turn it into an 
option). I'll make sure to implement (and hopefully test) a reasonable behavior 
for all various combinations of CMake flags (eg., clang-tidy enabled/disabled, 
static analyzer enabled/disabled, static-analyzer-into-clang-tidy integration 
enabled/disabled, etc.).

This patch introduces a frontend flag `-analyzer-tidy-checker=...` that accepts 
clang-tidy checker strings (eg., 
`-analyzer-tidy-checker=bugprone-*,-cert-*,cert-env33-c`). As long as at least 
one such flag is supplied, `ClangTidyContext` is instantiated with the given 
checker list and a clang-tidy AST consumer gets multiplexed with 
`AnalysisConsumer` so that to run clang-tidy checkers. Diagnostics from these 
checks are pumped into a `PathDiagnosticConverterDiagnosticConsumer` (D94476 
) and displayed similarly to static analyzer 
diagnostics, eg. as HTML or Plist reports:

F15183921: Screen Shot 2021-01-25 at 2.59.30 PM.png 


This patch suggests enabling a few clang-tidy checks by default in Clang Driver 
(under the --analyze flag, obviously, and the new reverse integration should be 
enabled). Requirements for enabling a clang-tidy check by default would be 
similar to requirements of enabling a static analyzer check by default (finds 
real bugs by design, understandable diagnostics, etc.). Additionally, 
on-by-default clang-tidy checks should have static analyzer bug categories 
assigned to them (eg., "Logic error", "Memory error", etc.) in the diagnostic 
converter (a relatively convenient way to do so is provided).

If bug categories aren't assigned, the default bug category for clang-tidy 
check `x-y-z` is `Clang-Tidy [x]` and the default bug type is `Clang-Tidy 
[x-y-z]`. This looks pretty ok in scan-build's index.html:

F15183900: Screen Shot 2021-01-25 at 2.56.47 PM.png 


That said, we should probably provide an option to disable bug category 
conversion and use it whenever an arbitrary clang-tidy config is expected. As 
usual, we're optimizing for a reasonable default rather than for 
configurability.

I've considered a different approach to enabling/disabling clang-tidy checks 
(but didn't implement it in this patch) that's more static analyzer-like: 
`-analyzer-checker tidy.bugprone.assert-side-effect` etc. Here we treat `x` in 
`x-y-z` as a static analyzer package, hence the dot. This approach is less 
flexible because it doesn't allow arbitrary wildcards, so we still have to have 
the current approach but we should probably implement both. One thing to think 
about here would be to figure out whether each individual off-by-default 
clang-tidy check is treated as "alpha" ("we're not supporting it, don't use 
it") or opt-in ("we're supporting it but it's still of by default"). Probably 
we could additionally separate them into `alpha.tidy.*` and `optin.tidy.*`.

Clang-tidy check-specific options are not implemented yet. Again, we could 
provide a separate frontend flag, or we could interface them through 
`AnalyzerOptions` like this: `-analyzer-config 
tidy.bugprone.assert-side-effect:AssertMacros=assert,Assert,ASSERT`. And, 
again, we should probably do both.


https://reviews.llvm.org/D95403

Files:
  clang/include/clang/Driver/Options.td
  clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
  clang/include/clang/StaticAnalyzer/Frontend/AnalysisConsumer.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
  clang/lib/StaticAnalyzer/Frontend/CMakeLists.txt

Index: clang/lib/StaticAnalyzer/Frontend/CMakeLists.txt
===
--- clang/lib/StaticAnalyzer/Frontend/CMakeLists.txt
+++ clang/lib/StaticAnalyzer/Frontend/CMakeLists.txt
@@ -1,3 +1,7 @@
+configure_file(
+  ${CMAKE_SOURCE_DIR}/../clang-tools-extra/clang-tidy/clang-tidy-config.h.cmake
+  ${CMAKE_CURRENT_BINARY_DIR}/clang-tidy-config.h)
+
 include_directories( ${CMAKE_CURRENT_BINARY_DIR}/../Checkers )
 
 set(LLVM_LINK_COMPONENTS
@@ -23,6 

[PATCH] D94627: [PowerPC][PC Rel] Implement option to omit P10 instructions from stubs

[Albion Fung via Phabricator via cfe-commits]

Conanap added a comment.

Addressed comments




Comment at: lld/ELF/Config.h:74
+// For --power10-stub
+enum class P10Stub { Default, No };
+

amyk wrote:
> We have a "yes", but does it need to be here, too?
After a bit of discussion, since we don't have a concrete implementation for 
the "yes" option yet, the consensus is to keep it out for now. I'll remove it 
from the help message as well.



Comment at: lld/ELF/Driver.cpp:776
+  }
+
+  return P10Stub::Default;

amyk wrote:
> Do we need to handle `power10_stubs`?
That option is handled by the variable `NoP10`; I'll add a comment to make that 
more clear.



Comment at: lld/ELF/Thunks.cpp:920
   const int64_t offset = computeOffset();
-  write32(buf + 0, 0xf8410018); // std  r2,24(r1)
+  write32(buf + 0, 0xf8410018);   // std  r2,24(r1)
   // The branch offset needs to fit in 26 bits.

amyk wrote:
> Unrelated change?
This change was made so that the commented instruction would better line up 
with the new instructions added for better readablity. 


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

https://reviews.llvm.org/D94627

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

[PATCH] D94627: [PowerPC][PC Rel] Implement option to omit P10 instructions from stubs

[Albion Fung via Phabricator via cfe-commits]

Conanap updated this revision to Diff 319140.
Conanap marked 7 inline comments as done.
Conanap added a comment.

Fixed some formatting and comments.


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

https://reviews.llvm.org/D94627

Files:
  lld/ELF/Config.h
  lld/ELF/Driver.cpp
  lld/ELF/Options.td
  lld/ELF/Thunks.cpp
  lld/test/ELF/ppc64-call-reach.s
  lld/test/ELF/ppc64-long-branch-localentry-offset.s
  lld/test/ELF/ppc64-long-branch-pi.s
  lld/test/ELF/ppc64-long-branch-rel14.s
  lld/test/ELF/ppc64-long-branch.s
  lld/test/ELF/ppc64-pcrel-call-to-extern.s
  lld/test/ELF/ppc64-pcrel-call-to-toc.s
  lld/test/ELF/ppc64-plt-stub-compatible.s
  lld/test/ELF/ppc64-tls-pcrel-gd.s
  lld/test/ELF/ppc64-toc-call-to-pcrel-long-jump.s
  lld/test/ELF/ppc64-toc-call-to-pcrel.s
  llvm/include/llvm/Object/ELF.h

Index: llvm/include/llvm/Object/ELF.h
===
--- llvm/include/llvm/Object/ELF.h
+++ llvm/include/llvm/Object/ELF.h
@@ -87,7 +87,11 @@
 
 enum PPCInstrMasks : uint64_t {
   PADDI_R12_NO_DISP = 0x06103980,
+  ADDIS_R12_NO_DISP = 0x3D80,
+  ADDI_R12_TO_R12_NO_DISP = 0x398C,
   PLD_R12_NO_DISP = 0x0410E580,
+  LD_R12_NO_DISP = 0xE980,
+  LD_R12_TO_R12_NO_DISP = 0xE98C,
   MTCTR_R12 = 0x7D8903A6,
   BCTR = 0x4E800420,
 };
Index: lld/test/ELF/ppc64-toc-call-to-pcrel.s
===
--- lld/test/ELF/ppc64-toc-call-to-pcrel.s
+++ lld/test/ELF/ppc64-toc-call-to-pcrel.s
@@ -14,6 +14,12 @@
 # RUN: llvm-readelf -s %t | FileCheck %s --check-prefix=SYMBOL
 # RUN: llvm-objdump -d --no-show-raw-insn --mcpu=future %t | FileCheck %s
 
+# RUN: llvm-mc -filetype=obj -triple=powerpc64 %s -o %t.o
+# RUN: ld.lld -T %t.script %t.o -o %t --no-power10-stubs
+# RUN: llvm-readelf -s %t | FileCheck %s --check-prefix=SYMBOL
+# RUN: llvm-objdump -d --no-show-raw-insn --mcpu=future %t \
+# RUN: | FileCheck %s
+
 # The point of this test is to make sure that when a function with TOC access
 # a local function with st_other=1, a TOC save stub is inserted.
 
Index: lld/test/ELF/ppc64-toc-call-to-pcrel-long-jump.s
===
--- lld/test/ELF/ppc64-toc-call-to-pcrel-long-jump.s
+++ lld/test/ELF/ppc64-toc-call-to-pcrel-long-jump.s
@@ -16,10 +16,10 @@
 # SYM-NEXT: 2: 20020008 0 NOTYPE  LOCAL  DEFAULT []   3 caller
 # SYM-NEXT: 3: 10020008 0 NOTYPE  LOCAL  DEFAULT  2 caller_close
 # SYM-NEXT: 4: 000520020008 0 NOTYPE  LOCAL  DEFAULT  4 caller_far
-# SYM-NEXT: 5: 000520028038 0 NOTYPE  LOCAL  HIDDEN   6 .TOC.
+# SYM-NEXT: 5: 000520028040 0 NOTYPE  LOCAL  HIDDEN   6 .TOC.
 # SYM-NEXT: 6: 10020020 8 FUNCLOCAL  DEFAULT  2 __toc_save_callee
-# SYM-NEXT: 7: 2002002020 FUNCLOCAL  DEFAULT  3 __toc_save_callee
-# SYM-NEXT: 8: 00052002002020 FUNCLOCAL  DEFAULT  4 __toc_save_callee
+# SYM-NEXT: 7: 2002002032 FUNCLOCAL  DEFAULT  3 __toc_save_callee
+# SYM-NEXT: 8: 00052002002032 FUNCLOCAL  DEFAULT  4 __toc_save_callee
 
 #--- lts
 PHDRS {
Index: lld/test/ELF/ppc64-tls-pcrel-gd.s
===
--- lld/test/ELF/ppc64-tls-pcrel-gd.s
+++ lld/test/ELF/ppc64-tls-pcrel-gd.s
@@ -42,10 +42,10 @@
 #--- asm
 
 # GD-RELOC: Relocation section '.rela.dyn' at offset 0x100b8 contains 4 entries:
-# GD-RELOC: 01001160  00020044 R_PPC64_DTPMOD64    x + 0
-# GD-RELOC: 01001168  0002004e R_PPC64_DTPREL64    x + 0
-# GD-RELOC: 01001170  00030044 R_PPC64_DTPMOD64    y + 0
-# GD-RELOC: 01001178  0003004e R_PPC64_DTPREL64    y + 0
+# GD-RELOC: 01001170  00020044 R_PPC64_DTPMOD64    x + 0
+# GD-RELOC: 01001178  0002004e R_PPC64_DTPREL64    x + 0
+# GD-RELOC: 01001180  00030044 R_PPC64_DTPMOD64    y + 0
+# GD-RELOC: 01001188  0003004e R_PPC64_DTPREL64    y + 0
 
 # GD-SYM:   Symbol table '.dynsym' contains 4 entries:
 # GD-SYM:   2:  0 TLS GLOBAL DEFAULT   UND x
@@ -68,9 +68,9 @@
 # GDTOLE-SYM: 4: 0004 0 TLS GLOBAL DEFAULT 3 y
 
 # GD-LABEL: :
-# GD-NEXT:paddi 3, 0, 352, 1
+# GD-NEXT:paddi 3, 0, 368, 1
 # GD-NEXT:bl
-# GD-NEXT:paddi 3, 0, 356, 1
+# GD-NEXT:paddi 3, 0, 372, 1
 # GD-NEXT:bl
 # GD-NEXT:blr
 # GDTOIE-LABEL: :
Index: lld/test/ELF/ppc64-plt-stub-compatible.s
===
--- lld/test/ELF/ppc64-plt-stub-compatible.s
+++ 

[PATCH] D90646: [clang] Add warning when `-include-pch` is passed multiple times

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

There are many options whether the latter one overrides the previous ones. How 
is this special?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90646

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

[PATCH] D90646: [clang] Add warning when `-include-pch` is passed multiple times

[Keith Smiley via Phabricator via cfe-commits]

keith updated this revision to Diff 319138.
keith added a comment.
Herald added a reviewer: jansvoboda11.

Add group to warning


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90646

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/PCH/multiple-include-pch-warning.c


Index: clang/test/PCH/multiple-include-pch-warning.c
===
--- /dev/null
+++ clang/test/PCH/multiple-include-pch-warning.c
@@ -0,0 +1,5 @@
+// RUN: %clang_cc1 -emit-pch -o %t1.pch %s
+// RUN: %clang_cc1 -emit-pch -o %t2.pch %s
+// RUN: %clang_cc1 %s -include-pch %t1.pch -include-pch %t2.pch 2>&1 | 
FileCheck %s
+
+// CHECK: warning: -include-pch passed multiple times but only the last 
'{{.*\.pch}}' is being used
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -2779,6 +2779,14 @@
 static void ParsePreprocessorArgs(PreprocessorOptions , ArgList ,
   DiagnosticsEngine ,
   frontend::ActionKind Action) {
+  auto allPchFiles = Args.getAllArgValues(OPT_include_pch);
+  if (!allPchFiles.empty()) {
+Opts.ImplicitPCHInclude = std::string(allPchFiles.back());
+if (allPchFiles.size() > 1)
+  Diags.Report(diag::warn_drv_multiple_include_pch)
+  << Opts.ImplicitPCHInclude;
+  }
+
   Opts.PCHWithHdrStop = Args.hasArg(OPT_pch_through_hdrstop_create) ||
 Args.hasArg(OPT_pch_through_hdrstop_use);
 
Index: clang/include/clang/Basic/DiagnosticDriverKinds.td
===
--- clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -332,6 +332,9 @@
   InGroup;
 def warn_drv_pch_not_first_include : Warning<
   "precompiled header '%0' was ignored because '%1' is not first '-include'">;
+def warn_drv_multiple_include_pch : Warning<
+  "-include-pch passed multiple times but only the last '%0' is being used">,
+   InGroup;
 def warn_missing_sysroot : Warning<"no such sysroot directory: '%0'">,
   InGroup>;
 def warn_incompatible_sysroot : Warning<"using sysroot for '%0' but targeting 
'%1'">,


Index: clang/test/PCH/multiple-include-pch-warning.c
===
--- /dev/null
+++ clang/test/PCH/multiple-include-pch-warning.c
@@ -0,0 +1,5 @@
+// RUN: %clang_cc1 -emit-pch -o %t1.pch %s
+// RUN: %clang_cc1 -emit-pch -o %t2.pch %s
+// RUN: %clang_cc1 %s -include-pch %t1.pch -include-pch %t2.pch 2>&1 | FileCheck %s
+
+// CHECK: warning: -include-pch passed multiple times but only the last '{{.*\.pch}}' is being used
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -2779,6 +2779,14 @@
 static void ParsePreprocessorArgs(PreprocessorOptions , ArgList ,
   DiagnosticsEngine ,
   frontend::ActionKind Action) {
+  auto allPchFiles = Args.getAllArgValues(OPT_include_pch);
+  if (!allPchFiles.empty()) {
+Opts.ImplicitPCHInclude = std::string(allPchFiles.back());
+if (allPchFiles.size() > 1)
+  Diags.Report(diag::warn_drv_multiple_include_pch)
+  << Opts.ImplicitPCHInclude;
+  }
+
   Opts.PCHWithHdrStop = Args.hasArg(OPT_pch_through_hdrstop_create) ||
 Args.hasArg(OPT_pch_through_hdrstop_use);
 
Index: clang/include/clang/Basic/DiagnosticDriverKinds.td
===
--- clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -332,6 +332,9 @@
   InGroup;
 def warn_drv_pch_not_first_include : Warning<
   "precompiled header '%0' was ignored because '%1' is not first '-include'">;
+def warn_drv_multiple_include_pch : Warning<
+  "-include-pch passed multiple times but only the last '%0' is being used">,
+   InGroup;
 def warn_missing_sysroot : Warning<"no such sysroot directory: '%0'">,
   InGroup>;
 def warn_incompatible_sysroot : Warning<"using sysroot for '%0' but targeting '%1'">,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D95279: Support: Remove duplicated code in {File,clang::ModulesDependency}Collector, NFC

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

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG080952a9447a: Support: Remove duplicated code in 
{File,clang::ModulesDependency}Collector, NFC (authored by dexonsmith).

Changed prior to commit:
  https://reviews.llvm.org/D95279?vs=318701=319136#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95279

Files:
  clang/include/clang/Frontend/Utils.h
  clang/lib/Frontend/ModuleDependencyCollector.cpp
  llvm/include/llvm/Support/FileCollector.h
  llvm/lib/Support/FileCollector.cpp
  llvm/unittests/Support/FileCollectorTest.cpp

Index: llvm/unittests/Support/FileCollectorTest.cpp
===
--- llvm/unittests/Support/FileCollectorTest.cpp
+++ llvm/unittests/Support/FileCollectorTest.cpp
@@ -33,7 +33,6 @@
   using FileCollector::FileCollector;
   using FileCollector::Root;
   using FileCollector::Seen;
-  using FileCollector::SymlinkMap;
   using FileCollector::VFSWriter;
 
   bool hasSeen(StringRef fs) {
Index: llvm/lib/Support/FileCollector.cpp
===
--- llvm/lib/Support/FileCollector.cpp
+++ llvm/lib/Support/FileCollector.cpp
@@ -53,62 +53,82 @@
 : Root(std::move(Root)), OverlayRoot(std::move(OverlayRoot)) {
 }
 
-bool FileCollector::getRealPath(StringRef SrcPath,
-SmallVectorImpl ) {
+void FileCollector::PathCanonicalizer::updateWithRealPath(
+SmallVectorImpl ) {
+  StringRef SrcPath(Path.begin(), Path.size());
+  StringRef Filename = sys::path::filename(SrcPath);
+  StringRef Directory = sys::path::parent_path(SrcPath);
+
+  // Use real_path to fix any symbolic link component present in the directory
+  // part of the path, caching the search because computing the real path is
+  // expensive.
   SmallString<256> RealPath;
-  StringRef FileName = sys::path::filename(SrcPath);
-  std::string Directory = sys::path::parent_path(SrcPath).str();
-  auto DirWithSymlink = SymlinkMap.find(Directory);
-
-  // Use real_path to fix any symbolic link component present in a path.
-  // Computing the real path is expensive, cache the search through the parent
-  // path Directory.
-  if (DirWithSymlink == SymlinkMap.end()) {
-auto EC = sys::fs::real_path(Directory, RealPath);
-if (EC)
-  return false;
-SymlinkMap[Directory] = std::string(RealPath.str());
+  auto DirWithSymlink = CachedDirs.find(Directory);
+  if (DirWithSymlink == CachedDirs.end()) {
+// FIXME: Should this be a call to FileSystem::getRealpath(), in some
+// cases? What if there is nothing on disk?
+if (sys::fs::real_path(Directory, RealPath))
+  return;
+CachedDirs[Directory] = std::string(RealPath.str());
   } else {
 RealPath = DirWithSymlink->second;
   }
 
-  sys::path::append(RealPath, FileName);
-  Result.swap(RealPath);
-  return true;
+  // Finish recreating the path by appending the original filename, since we
+  // don't need to resolve symlinks in the filename.
+  //
+  // FIXME: If we can cope with this, maybe we can cope without calling
+  // getRealPath() at all when there's no ".." component.
+  sys::path::append(RealPath, Filename);
+
+  // Swap to create the output.
+  Path.swap(RealPath);
 }
 
-void FileCollector::addFileImpl(StringRef SrcPath) {
+/// Make Path absolute.
+static void makeAbsolute(SmallVectorImpl ) {
   // We need an absolute src path to append to the root.
-  SmallString<256> AbsoluteSrc = SrcPath;
-  sys::fs::make_absolute(AbsoluteSrc);
+  sys::fs::make_absolute(Path);
 
   // Canonicalize src to a native path to avoid mixed separator styles.
-  sys::path::native(AbsoluteSrc);
+  sys::path::native(Path);
 
   // Remove redundant leading "./" pieces and consecutive separators.
-  StringRef TrimmedAbsoluteSrc =
-  sys::path::remove_leading_dotslash(AbsoluteSrc);
+  Path.erase(Path.begin(), sys::path::remove_leading_dotslash(
+   StringRef(Path.begin(), Path.size()))
+   .begin());
+}
 
-  // Canonicalize the source path by removing "..", "." components.
-  SmallString<256> VirtualPath = TrimmedAbsoluteSrc;
-  sys::path::remove_dots(VirtualPath, /*remove_dot_dot=*/true);
+FileCollector::PathCanonicalizer::PathStorage
+FileCollector::PathCanonicalizer::canonicalize(StringRef SrcPath) {
+  PathStorage Paths;
+  Paths.VirtualPath = SrcPath;
+  makeAbsolute(Paths.VirtualPath);
 
   // If a ".." component is present after a symlink component, remove_dots may
   // lead to the wrong real destination path. Let the source be canonicalized
   // like that but make sure we always use the real path for the destination.
-  SmallString<256> CopyFrom;
-  if (!getRealPath(TrimmedAbsoluteSrc, CopyFrom))
-CopyFrom = VirtualPath;
+  Paths.CopyFrom = Paths.VirtualPath;
+  

[PATCH] D95017: [clang-format] add case aware include sorting

[Kent Sommer via Phabricator via cfe-commits]

kentsommer added inline comments.



Comment at: clang/docs/ClangFormatStyleOptions.rst:2286
+**IncludeSortAlphabetically** (``bool``)
+  Specify if sorting should be done in an alphabetical and
+  case sensitive fashion.

MyDeveloperDay wrote:
> Are you sure `IncludeSortAlphabetically` expresses what you mean? Once these 
> things get released they cannot be changed easily.
> 
> If you were not sure of a new option, in my view we should slow down and make 
> sure we have the correct design, for example you could have used a enum and 
> it might have given you more possibility for greater flexibility
> 
> ```
> enum IncludeSort
> {
>CaseInsensitive
>CaseSensitive
> }
> ```
> 
> Please give people time to re-review your changes before we commit, 
> especially if they've taken the time to look at your review in the first 
> place. Just saying.
> 
Hi, @MyDeveloperDay I definitely agree. It was not my intention to rush through 
the review. I was simply trying to follow the process outlined in 
https://llvm.org/docs/Contributing.html#how-to-submit-a-patch which mentions 
giving sufficient information to allow for a commit on your behalf when you 
don't have access after an LGTM (which is all that I did). As you can see from 
the lack of additional comments from my end, I was happy to let this sit and be 
reviewed. 

Per the discussion about the option itself, I do believe 
`IncludeSortAlphabetically` currently expresses what I mean as the behavior 
with this off is indeed not an alphabetical sort as case takes precedence over 
the alphabetical ordering. However, looking at the enum and realizing that 
others probably will have additional styles they prefer (maybe they want 
alphabetical but lower case first, etc.) I do believe it might have been a 
better way to go as it leaves more flexibility and room for additional ordering 
styles. Given that this just landed, I would be happy to open a patch to turn 
this into an `enum` as I do see benefits to doing so. What do you think?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95017

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

[PATCH] D94820: Support for instrumenting only selected files or functions

[David Li via Phabricator via cfe-commits]

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

my bad -- I missed the test case.  LGTM


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

https://reviews.llvm.org/D94820

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

[PATCH] D94745: [OpenMP][WIP] Build the deviceRTLs with OpenMP instead of target dependent language - NOT FOR REVIEW

[Shilei Tian via Phabricator via cfe-commits]

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

Fixed failed driver test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94745

Files:
  clang/lib/Driver/ToolChains/Cuda.cpp
  clang/test/Driver/Inputs/libomptarget/libomptarget-nvptx-cuda_80-sm_20.bc
  clang/test/Driver/Inputs/libomptarget/libomptarget-nvptx-sm_20.bc
  openmp/libomptarget/deviceRTLs/common/allocator.h
  openmp/libomptarget/deviceRTLs/common/omptarget.h
  openmp/libomptarget/deviceRTLs/common/src/libcall.cu
  openmp/libomptarget/deviceRTLs/common/src/omp_data.cu
  openmp/libomptarget/deviceRTLs/common/src/reduction.cu
  openmp/libomptarget/deviceRTLs/nvptx/CMakeLists.txt
  openmp/libomptarget/deviceRTLs/nvptx/src/nvptx_interface.h
  openmp/libomptarget/deviceRTLs/nvptx/src/target_impl.cu
  openmp/libomptarget/deviceRTLs/nvptx/src/target_impl.h

Index: openmp/libomptarget/deviceRTLs/nvptx/src/target_impl.h
===
--- openmp/libomptarget/deviceRTLs/nvptx/src/target_impl.h
+++ openmp/libomptarget/deviceRTLs/nvptx/src/target_impl.h
@@ -13,18 +13,16 @@
 #define _TARGET_IMPL_H_
 
 #include 
-#include 
 #include 
 #include 
 #include 
 
 #include "nvptx_interface.h"
 
-#define DEVICE __device__
-#define INLINE __forceinline__ DEVICE
-#define NOINLINE __noinline__ DEVICE
-#define SHARED __shared__
-#define ALIGN(N) __align__(N)
+#define DEVICE
+#define INLINE inline __attribute__((always_inline))
+#define NOINLINE __attribute__((noinline))
+#define ALIGN(N) __attribute__((aligned(N)))
 
 
 // Kernel options
Index: openmp/libomptarget/deviceRTLs/nvptx/src/target_impl.cu
===
--- openmp/libomptarget/deviceRTLs/nvptx/src/target_impl.cu
+++ openmp/libomptarget/deviceRTLs/nvptx/src/target_impl.cu
@@ -14,8 +14,6 @@
 #include "target_impl.h"
 #include "common/debug.h"
 
-#include 
-
 DEVICE void __kmpc_impl_unpack(uint64_t val, uint32_t , uint32_t ) {
   asm volatile("mov.b64 {%0,%1}, %2;" : "=r"(lo), "=r"(hi) : "l"(val));
 }
Index: openmp/libomptarget/deviceRTLs/nvptx/src/nvptx_interface.h
===
--- openmp/libomptarget/deviceRTLs/nvptx/src/nvptx_interface.h
+++ openmp/libomptarget/deviceRTLs/nvptx/src/nvptx_interface.h
@@ -11,7 +11,8 @@
 
 #include 
 
-#define EXTERN extern "C" __device__
+#define EXTERN extern "C"
+
 typedef uint32_t __kmpc_impl_lanemask_t;
 typedef uint32_t omp_lock_t; /* arbitrary type of the right length */
 
Index: openmp/libomptarget/deviceRTLs/nvptx/CMakeLists.txt
===
--- openmp/libomptarget/deviceRTLs/nvptx/CMakeLists.txt
+++ openmp/libomptarget/deviceRTLs/nvptx/CMakeLists.txt
@@ -10,6 +10,21 @@
 #
 ##===--===##
 
+# TODO: This part needs to be refined when libomptarget is going to support
+# Windows!
+# TODO: This part can also be removed if we can change the clang driver to make
+# it support device only compilation.
+if(CMAKE_HOST_SYSTEM_PROCESSOR MATCHES "x86_64")
+  set(aux_triple x86_64-unknown-linux-gnu)
+elseif(CMAKE_HOST_SYSTEM_PROCESSOR MATCHES "ppc64le")
+  set(aux_triple powerpc64le-unknown-linux-gnu)
+elseif(CMAKE_HOST_SYSTEM_PROCESSOR MATCHES "aarch64")
+  set(aux_triple aarch64-unknown-linux-gnu)
+else()
+  libomptarget_say("Not building CUDA offloading device RTL: unknown host arch: ${CMAKE_HOST_SYSTEM_PROCESSOR}")
+  return()
+endif()
+
 get_filename_component(devicertl_base_directory
   ${CMAKE_CURRENT_SOURCE_DIR}
   DIRECTORY)
@@ -79,61 +94,82 @@
 )
 
 # Set flags for LLVM Bitcode compilation.
-set(bc_flags ${LIBOMPTARGET_NVPTX_SELECTED_CUDA_COMPILER_FLAGS}
+set(bc_flags -S -x c++
+ -target nvptx64
+ -Xclang -emit-llvm-bc
+ -Xclang -aux-triple -Xclang ${aux_triple}
+ -fopenmp -fopenmp-cuda-mode -Xclang -fopenmp-is-device
+ -D__CUDACC__
  -I${devicertl_base_directory}
  -I${devicertl_nvptx_directory}/src)
 
 if(${LIBOMPTARGET_NVPTX_DEBUG})
-  set(bc_flags ${bc_flags} -DOMPTARGET_NVPTX_DEBUG=-1)
+  list(APPEND bc_flags -DOMPTARGET_NVPTX_DEBUG=-1)
 else()
-  set(bc_flags ${bc_flags} -DOMPTARGET_NVPTX_DEBUG=0)
+  list(APPEND bc_flags -DOMPTARGET_NVPTX_DEBUG=0)
 endif()
 
 # Create target to build all Bitcode libraries.
 add_custom_target(omptarget-nvptx-bc)
 
-# Generate a Bitcode library for all the compute capabilities the user requested.
+# This correlation is from clang/lib/Driver/ToolChains/Cuda.cpp.
+# The last element is the default case.
+set(cuda_version_list 110 102 101 100 92 91 90 80)
+set(ptx_feature_list 70 65 

[PATCH] D95253: [clang][Fuchsia] Add relative-vtables + asan multilibs

[Petr Hosek via Phabricator via cfe-commits]

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

LGTM




Comment at: clang/lib/Driver/ToolChains/Fuchsia.cpp:220
   .flag("+fno-exceptions"));
+  Multilibs.push_back(Multilib("relative-vtables+asan", {}, {}, 6)
+  .flag("+fexperimental-relative-c++-abi-vtables")

Could you leave a `TODO` to remove this once relative vtables are the default 
for Fuchsia?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95253

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

[PATCH] D94820: Support for instrumenting only selected files or functions

[Petr Hosek via Phabricator via cfe-commits]

phosek added inline comments.



Comment at: clang/test/CodeGen/profile-filter.c:17
+// RUN: %clang_cc1 -fprofile-instrument=clang 
-fprofile-list=%t-exclude-only.list -emit-llvm %s -o - | FileCheck %s 
--check-prefix=EXCLUDE
+
+unsigned i;

davidxl wrote:
> phosek wrote:
> > davidxl wrote:
> > > Can you add a test case with both include and exclude list (the 
> > > intersection of INCLUDE_SET & !EXCLUDE_SET)?
> > See the case above this one which has:
> > ```
> > fun:test*
> > !fun:test1
> > ```
> > Is that what you have in mind?
> Right
Do we need to do anything then since this test case already exists?


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

https://reviews.llvm.org/D94820

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

[PATCH] D95396: Improve static_assert/_Static_assert diagnostics

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman created this revision.
aaron.ballman added reviewers: rsmith, jyknight.
aaron.ballman requested review of this revision.

I noticed that our diagnostics relating to static assertions are a bit 
confused. For instance, when in MS compatibility mode in C (where we accept 
`static_assert` even without including ``), we would fail to warn the 
user that they were using the wrong spelling (even in pedantic mode), we were 
missing a compatibility warning about using `_Static_assert` in earlier 
standards modes, diagnostics for the optional message were not reflected in C 
as they were in C++, etc.

This patch improves the diagnostic functionality in both C and C++ mode. It 
adds `-Wc89-c99-c11-c17-compat` and `-Wc89-c99-c11-c17-compat-pedantic` 
diagnostics groups and adds new C-specific diagnostics for a static assertion 
without a diagnostic message.


https://reviews.llvm.org/D95396

Files:
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/test/Parser/static_assert.c
  clang/test/SemaCXX/static-assert.cpp

Index: clang/test/SemaCXX/static-assert.cpp
===
--- clang/test/SemaCXX/static-assert.cpp
+++ clang/test/SemaCXX/static-assert.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s -std=c++11 -triple=x86_64-linux-gnu
+// RUN: %clang_cc1 -fsyntax-only -verify %s -std=c++11 -pedantic -triple=x86_64-linux-gnu
 
 int f(); // expected-note {{declared here}}
 
Index: clang/test/Parser/static_assert.c
===
--- /dev/null
+++ clang/test/Parser/static_assert.c
@@ -0,0 +1,45 @@
+// RUN: %clang_cc1 -fsyntax-only -std=c2x -DTEST_SPELLING -verify=c2x %s
+// RUN: %clang_cc1 -fsyntax-only -std=c2x -DTEST_SPELLING -fms-compatibility -verify=c2x-ms %s
+// RUN: %clang_cc1 -fsyntax-only -std=c2x -Wc89-c99-c11-c17-compat -verify=c2x-compat %s
+// RUN: %clang_cc1 -fsyntax-only -std=c99 -verify=c99 %s
+// RUN: %clang_cc1 -fsyntax-only -std=c99 -pedantic -verify=c99-pedantic %s
+// RUN: %clang_cc1 -fsyntax-only -std=c++17 -verify=cxx17 -x c++ %s
+// RUN: %clang_cc1 -fsyntax-only -std=c++17 -pedantic -verify=cxx17-pedantic -x c++ %s
+// RUN: %clang_cc1 -fsyntax-only -std=c++98 -verify=cxx98 -x c++ %s
+// RUN: %clang_cc1 -fsyntax-only -std=c++98 -pedantic -verify=cxx98-pedantic -x c++ %s
+// RUN: %clang_cc1 -fsyntax-only -std=c++17 -Wc++98-c++11-c++14-compat -verify=cxx17-compat -x c++ %s
+
+// c99-no-diagnostics
+// cxx17-no-diagnostics
+// cxx98-no-diagnostics
+
+#ifdef TEST_SPELLING
+// Only test the C++ spelling in C mode in some of the tests, to reduce the
+// amount of diagnostics to have to check. This spelling is allowed in MS-
+// compatibility mode in C, but otherwise produces errors.
+static_assert(1, ""); // c2x-error {{expected parameter declarator}} \
+  // c2x-error {{expected ')'}} \
+  // c2x-note {{to match this '('}} \
+  // c2x-warning {{type specifier missing, defaults to 'int'}} \
+  // c2x-ms-warning {{'static_assert' is spelled '_Static_assert' in C; consider including }}
+#endif
+
+// We support _Static_assert as an extension in older C modes and in all C++
+// modes, but only as a pedantic warning.
+_Static_assert(1, ""); // c99-pedantic-warning {{'_Static_assert' is a C11 extension}} \
+   // cxx17-pedantic-warning {{'_Static_assert' is a C11 extension}} \
+   // cxx98-pedantic-warning {{'_Static_assert' is a C11 extension}}
+
+// _Static_assert without a message has more complex diagnostic logic:
+//   * In C++17 or C2x mode, it's supported by default.
+//   * But there is a special compat warning flag to warn about portability to
+// older standards.
+//   * In older standard pedantic modes, warn about supporting without a
+// message as an extension.
+_Static_assert(1); // c99-pedantic-warning {{'_Static_assert' with no message is a C2x extension}} \
+   // cxx98-pedantic-warning {{'static_assert' with no message is a C++17 extension}} \
+   // c2x-compat-warning {{'_Static_assert' with no message is incompatible with C standards before C2x}} \
+   // cxx17-compat-warning {{'static_assert' with no message is incompatible with C++ standards before C++17}} \
+   // c99-pedantic-warning {{'_Static_assert' is a C11 extension}} \
+   // cxx17-pedantic-warning {{'_Static_assert' is a C11 extension}} \
+   // cxx98-pedantic-warning {{'_Static_assert' is a C11 extension}}
Index: clang/lib/Parse/ParseDeclCXX.cpp
===
--- clang/lib/Parse/ParseDeclCXX.cpp
+++ clang/lib/Parse/ParseDeclCXX.cpp
@@ -871,8 +871,13 @@
 
   if (Tok.is(tok::kw__Static_assert) && !getLangOpts().C11)
 Diag(Tok, 

[PATCH] D95392: Restore GNU , ## __VA_ARGS__ behavior in MSVC mode

[Harald van Dijk via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGb43c26d036dc: Restore GNU , ## __VA_ARGS__ behavior in MSVC 
mode (authored by hvdijk).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95392

Files:
  clang/lib/Lex/TokenLexer.cpp
  clang/test/Preprocessor/macro_fn_comma_swallow2.c


Index: clang/test/Preprocessor/macro_fn_comma_swallow2.c
===
--- clang/test/Preprocessor/macro_fn_comma_swallow2.c
+++ clang/test/Preprocessor/macro_fn_comma_swallow2.c
@@ -9,6 +9,8 @@
 // RUN: %clang_cc1 -E -x c++ -std=c++11 %s | FileCheck -check-prefix=C99 
-strict-whitespace %s
 // RUN: %clang_cc1 -E -std=gnu99 %s | FileCheck -check-prefix=GCC 
-strict-whitespace %s
 // RUN: %clang_cc1 -E -fms-compatibility %s | FileCheck -check-prefix=MS 
-strict-whitespace %s
+// RUN: %clang_cc1 -E -x c++ -fms-compatibility %s | FileCheck 
-check-prefix=MS -strict-whitespace %s
+// RUN: %clang_cc1 -E -x c++ -std=c++11 -fms-compatibility %s | FileCheck 
-check-prefix=MS -strict-whitespace %s
 // RUN: %clang_cc1 -E -DNAMED %s | FileCheck -check-prefix=GCC 
-strict-whitespace %s
 // RUN: %clang_cc1 -E -std=c99 -DNAMED %s | FileCheck -check-prefix=C99 
-strict-whitespace %s
 
Index: clang/lib/Lex/TokenLexer.cpp
===
--- clang/lib/Lex/TokenLexer.cpp
+++ clang/lib/Lex/TokenLexer.cpp
@@ -152,7 +152,8 @@
   // named arguments, where it remains.  With GNU extensions, it is removed
   // regardless of named arguments.
   // Microsoft also appears to support this extension, unofficially.
-  if (!PP.getLangOpts().GNUMode && Macro->getNumParams() < 2)
+  if (!PP.getLangOpts().GNUMode && !PP.getLangOpts().MSVCCompat &&
+  Macro->getNumParams() < 2)
 return false;
 
   // Is a comma available to be removed?


Index: clang/test/Preprocessor/macro_fn_comma_swallow2.c
===
--- clang/test/Preprocessor/macro_fn_comma_swallow2.c
+++ clang/test/Preprocessor/macro_fn_comma_swallow2.c
@@ -9,6 +9,8 @@
 // RUN: %clang_cc1 -E -x c++ -std=c++11 %s | FileCheck -check-prefix=C99 -strict-whitespace %s
 // RUN: %clang_cc1 -E -std=gnu99 %s | FileCheck -check-prefix=GCC -strict-whitespace %s
 // RUN: %clang_cc1 -E -fms-compatibility %s | FileCheck -check-prefix=MS -strict-whitespace %s
+// RUN: %clang_cc1 -E -x c++ -fms-compatibility %s | FileCheck -check-prefix=MS -strict-whitespace %s
+// RUN: %clang_cc1 -E -x c++ -std=c++11 -fms-compatibility %s | FileCheck -check-prefix=MS -strict-whitespace %s
 // RUN: %clang_cc1 -E -DNAMED %s | FileCheck -check-prefix=GCC -strict-whitespace %s
 // RUN: %clang_cc1 -E -std=c99 -DNAMED %s | FileCheck -check-prefix=C99 -strict-whitespace %s
 
Index: clang/lib/Lex/TokenLexer.cpp
===
--- clang/lib/Lex/TokenLexer.cpp
+++ clang/lib/Lex/TokenLexer.cpp
@@ -152,7 +152,8 @@
   // named arguments, where it remains.  With GNU extensions, it is removed
   // regardless of named arguments.
   // Microsoft also appears to support this extension, unofficially.
-  if (!PP.getLangOpts().GNUMode && Macro->getNumParams() < 2)
+  if (!PP.getLangOpts().GNUMode && !PP.getLangOpts().MSVCCompat &&
+  Macro->getNumParams() < 2)
 return false;
 
   // Is a comma available to be removed?
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D95057: [clangd] Allow configuration database to be specified in config.

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG118c33ef47ac: [clangd] Allow configuration database to be 
specified in config. (authored by sammccall).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95057

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/Config.h
  clang-tools-extra/clangd/ConfigCompile.cpp
  clang-tools-extra/clangd/ConfigFragment.h
  clang-tools-extra/clangd/ConfigYAML.cpp
  clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
  clang-tools-extra/clangd/GlobalCompilationDatabase.h
  clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
  clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
  clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp

Index: clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp
===
--- clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp
+++ clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp
@@ -8,6 +8,7 @@
 
 #include "GlobalCompilationDatabase.h"
 
+#include "Config.h"
 #include "Matchers.h"
 #include "TestFS.h"
 #include "support/Path.h"
@@ -205,10 +206,12 @@
   llvm::formatv(CDBOuter, llvm::sys::path::convert_to_slash(testRoot()));
   FS.Files[testPath("build/compile_commands.json")] =
   llvm::formatv(CDBInner, llvm::sys::path::convert_to_slash(testRoot()));
+  FS.Files[testPath("foo/compile_flags.txt")] = "-DFOO";
 
   // Note that gen2.cc goes missing with our following model, not sure this
   // happens in practice though.
   {
+SCOPED_TRACE("Default ancestor scanning");
 DirectoryBasedGlobalCompilationDatabase DB(FS);
 std::vector DiscoveredFiles;
 auto Sub =
@@ -227,8 +230,53 @@
 EXPECT_THAT(DiscoveredFiles, UnorderedElementsAre(EndsWith("gen.cc")));
   }
 
-  // With a custom compile commands dir.
   {
+SCOPED_TRACE("With config");
+DirectoryBasedGlobalCompilationDatabase::Options Opts(FS);
+Opts.ContextProvider = [&](llvm::StringRef Path) {
+  Config Cfg;
+  if (Path.endswith("a.cc")) {
+// a.cc uses another directory's CDB, so it won't be discovered.
+Cfg.CompileFlags.CDBSearch.Policy = Config::CDBSearchSpec::FixedDir;
+Cfg.CompileFlags.CDBSearch.FixedCDBPath = testPath("foo");
+  } else if (Path.endswith("gen.cc")) {
+// gen.cc has CDB search disabled, so it won't be discovered.
+Cfg.CompileFlags.CDBSearch.Policy = Config::CDBSearchSpec::NoCDBSearch;
+  } else if (Path.endswith("gen2.cc")) {
+// gen2.cc explicitly lists this directory, so it will be discovered.
+Cfg.CompileFlags.CDBSearch.Policy = Config::CDBSearchSpec::FixedDir;
+Cfg.CompileFlags.CDBSearch.FixedCDBPath = testRoot();
+  }
+  return Context::current().derive(Config::Key, std::move(Cfg));
+};
+DirectoryBasedGlobalCompilationDatabase DB(Opts);
+std::vector DiscoveredFiles;
+auto Sub =
+DB.watch([](const std::vector Changes) {
+  DiscoveredFiles = Changes;
+});
+
+// Does not use the root CDB, so no broadcast.
+auto Cmd = DB.getCompileCommand(testPath("build/../a.cc"));
+ASSERT_TRUE(Cmd.hasValue());
+EXPECT_THAT(Cmd->CommandLine, Contains("-DFOO")) << "a.cc uses foo/ CDB";
+ASSERT_TRUE(DB.blockUntilIdle(timeoutSeconds(10)));
+EXPECT_THAT(DiscoveredFiles, IsEmpty()) << "Root CDB not discovered yet";
+
+// No special config for b.cc, so we trigger broadcast of the root CDB.
+DB.getCompileCommand(testPath("b.cc"));
+ASSERT_TRUE(DB.blockUntilIdle(timeoutSeconds(10)));
+EXPECT_THAT(DiscoveredFiles, ElementsAre(testPath("build/gen2.cc")));
+DiscoveredFiles.clear();
+
+// No CDB search so no discovery/broadcast triggered for build/ CDB.
+DB.getCompileCommand(testPath("build/gen.cc"));
+ASSERT_TRUE(DB.blockUntilIdle(timeoutSeconds(10)));
+EXPECT_THAT(DiscoveredFiles, IsEmpty());
+  }
+
+  {
+SCOPED_TRACE("With custom compile commands dir");
 DirectoryBasedGlobalCompilationDatabase::Options Opts(FS);
 Opts.CompileCommandsDir = testRoot();
 DirectoryBasedGlobalCompilationDatabase DB(Opts);
@@ -294,6 +342,58 @@
   EXPECT_EQ(testPath("x"), Commands.getValue().Directory);
 }
 
+MATCHER_P(hasArg, Flag, "") {
+  if (!arg.hasValue()) {
+*result_listener << "command is null";
+return false;
+  }
+  if (!llvm::is_contained(arg->CommandLine, Flag)) {
+*result_listener << "flags are " << llvm::join(arg->CommandLine, " ");
+return false;
+  }
+  return true;
+}
+
+TEST(GlobalCompilationDatabaseTest, Config) {
+  MockFS FS;
+  FS.Files[testPath("x/compile_flags.txt")] = "-DX";
+  FS.Files[testPath("x/y/z/compile_flags.txt")] = "-DZ";
+
+  Config::CDBSearchSpec Spec;
+  DirectoryBasedGlobalCompilationDatabase::Options Opts(FS);
+  

[PATCH] D95017: [clang-format] add case aware include sorting

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added inline comments.



Comment at: clang/docs/ClangFormatStyleOptions.rst:2286
+**IncludeSortAlphabetically** (``bool``)
+  Specify if sorting should be done in an alphabetical and
+  case sensitive fashion.

Are you sure `IncludeSortAlphabetically` expresses what you mean? Once these 
things get released they cannot be changed easily.

If you were not sure of a new option, in my view we should slow down and make 
sure we have the correct design, for example you could have used a enum and it 
might have given you more possibility for greater flexibility

```
enum IncludeSort
{
   CaseInsensitive
   CaseSensitive
}
```

Please give people time to re-review your changes before we commit, especially 
if they've taken the time to look at your review in the first place. Just 
saying.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95017

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

[PATCH] D94875: [clangd] ignore parallelism level for quick tasks

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG3680cb99a770: [clangd] ignore parallelism level for quick 
tasks (authored by qchateau, committed by sammccall).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94875

Files:
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/TUScheduler.cpp
  clang-tools-extra/clangd/TUScheduler.h


Index: clang-tools-extra/clangd/TUScheduler.h
===
--- clang-tools-extra/clangd/TUScheduler.h
+++ clang-tools-extra/clangd/TUScheduler.h
@@ -247,6 +247,13 @@
   void run(llvm::StringRef Name, llvm::StringRef Path,
llvm::unique_function Action);
 
+  /// Similar to run, except the task is expected to be quick.
+  /// This function will not honor AsyncThreadsCount (except
+  /// if threading is disabled with AsyncThreadsCount=0)
+  /// It is intended to run quick tasks that need to run ASAP
+  void runQuick(llvm::StringRef Name, llvm::StringRef Path,
+llvm::unique_function Action);
+
   /// Defines how a runWithAST action is implicitly cancelled by other actions.
   enum ASTActionInvalidation {
 /// The request will run unless explicitly cancelled.
@@ -319,10 +326,14 @@
   void profile(MemoryTree ) const;
 
 private:
+  void runWithSemaphore(llvm::StringRef Name, llvm::StringRef Path,
+llvm::unique_function Action, Semaphore );
+
   const GlobalCompilationDatabase 
   Options Opts;
   std::unique_ptr Callbacks; // not nullptr
   Semaphore Barrier;
+  Semaphore QuickRunBarrier;
   llvm::StringMap> Files;
   std::unique_ptr IdleASTs;
   // None when running tasks synchronously and non-None when running tasks
Index: clang-tools-extra/clangd/TUScheduler.cpp
===
--- clang-tools-extra/clangd/TUScheduler.cpp
+++ clang-tools-extra/clangd/TUScheduler.cpp
@@ -1265,7 +1265,7 @@
 : CDB(CDB), Opts(Opts),
   Callbacks(Callbacks ? move(Callbacks)
   : std::make_unique()),
-  Barrier(Opts.AsyncThreadsCount),
+  Barrier(Opts.AsyncThreadsCount), QuickRunBarrier(Opts.AsyncThreadsCount),
   IdleASTs(
   std::make_unique(Opts.RetentionPolicy.MaxRetainedASTs)) {
   // Avoid null checks everywhere.
@@ -1339,14 +1339,27 @@
 
 void TUScheduler::run(llvm::StringRef Name, llvm::StringRef Path,
   llvm::unique_function Action) {
+  runWithSemaphore(Name, Path, std::move(Action), Barrier);
+}
+
+void TUScheduler::runQuick(llvm::StringRef Name, llvm::StringRef Path,
+   llvm::unique_function Action) {
+  // Use QuickRunBarrier to serialize quick tasks: we are ignoring
+  // the parallelism level set by the user, don't abuse it
+  runWithSemaphore(Name, Path, std::move(Action), QuickRunBarrier);
+}
+
+void TUScheduler::runWithSemaphore(llvm::StringRef Name, llvm::StringRef Path,
+   llvm::unique_function Action,
+   Semaphore ) {
   if (!PreambleTasks) {
 WithContext WithProvidedContext(Opts.ContextProvider(Path));
 return Action();
   }
-  PreambleTasks->runAsync(Name, [this, Ctx = Context::current().clone(),
+  PreambleTasks->runAsync(Name, [this, , Ctx = Context::current().clone(),
  Path(Path.str()),
  Action = std::move(Action)]() mutable {
-std::lock_guard BarrierLock(Barrier);
+std::lock_guard BarrierLock(Sem);
 WithContext WC(std::move(Ctx));
 WithContext WithProvidedContext(Opts.ContextProvider(Path));
 Action();
Index: clang-tools-extra/clangd/ClangdServer.cpp
===
--- clang-tools-extra/clangd/ClangdServer.cpp
+++ clang-tools-extra/clangd/ClangdServer.cpp
@@ -407,7 +407,7 @@
   Result.push_back(replacementToEdit(Code, R));
 return CB(Result);
   };
-  WorkScheduler.run("FormatOnType", File, std::move(Action));
+  WorkScheduler.runQuick("FormatOnType", File, std::move(Action));
 }
 
 void ClangdServer::prepareRename(PathRef File, Position Pos,
@@ -635,7 +635,7 @@
 tooling::calculateRangesAfterReplacements(IncludeReplaces, Ranges),
 File)));
   };
-  WorkScheduler.run("Format", File, std::move(Action));
+  WorkScheduler.runQuick("Format", File, std::move(Action));
 }
 
 void ClangdServer::findDocumentHighlights(


Index: clang-tools-extra/clangd/TUScheduler.h
===
--- clang-tools-extra/clangd/TUScheduler.h
+++ clang-tools-extra/clangd/TUScheduler.h
@@ -247,6 +247,13 @@
   void run(llvm::StringRef Name, llvm::StringRef Path,
llvm::unique_function Action);
 
+  /// Similar to run, except the task is expected to be quick.
+  /// This function will not honor AsyncThreadsCount 

[PATCH] D94820: Support for instrumenting only selected files or functions

[David Li via Phabricator via cfe-commits]

davidxl added inline comments.



Comment at: clang/test/CodeGen/profile-filter.c:17
+// RUN: %clang_cc1 -fprofile-instrument=clang 
-fprofile-list=%t-exclude-only.list -emit-llvm %s -o - | FileCheck %s 
--check-prefix=EXCLUDE
+
+unsigned i;

phosek wrote:
> davidxl wrote:
> > Can you add a test case with both include and exclude list (the 
> > intersection of INCLUDE_SET & !EXCLUDE_SET)?
> See the case above this one which has:
> ```
> fun:test*
> !fun:test1
> ```
> Is that what you have in mind?
Right


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

https://reviews.llvm.org/D94820

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

[PATCH] D94820: Support for instrumenting only selected files or functions

[Petr Hosek via Phabricator via cfe-commits]

phosek added inline comments.



Comment at: clang/test/CodeGen/profile-filter.c:17
+// RUN: %clang_cc1 -fprofile-instrument=clang 
-fprofile-list=%t-exclude-only.list -emit-llvm %s -o - | FileCheck %s 
--check-prefix=EXCLUDE
+
+unsigned i;

davidxl wrote:
> Can you add a test case with both include and exclude list (the intersection 
> of INCLUDE_SET & !EXCLUDE_SET)?
See the case above this one which has:
```
fun:test*
!fun:test1
```
Is that what you have in mind?


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

https://reviews.llvm.org/D94820

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

[PATCH] D92808: [ObjC][ARC] Annotate calls with attributes instead of emitting retainRV or claimRV calls in the IR

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

Reverted in rG925ae8c790c7e354f12ec14a6cac6aa49fc75b29 
 for now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92808

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

[PATCH] D91913: Suppress non-conforming GNU paste extension in all standard-conforming modes

[Harald van Dijk via Phabricator via cfe-commits]

hvdijk added a comment.

In D91913#2521031 , @thakis wrote:

> We (and every other project out there) needs some incremental rollout plan 
> for this.

Cut the hyperbole please, this change does not affect "every other project out 
there". This change makes clang match GCC's 15+-year behaviour that other 
projects out there will have come to expect. I find it baffling that 
Chrome/Chromium developers would rely on a bug, knowing full well it's a bug 
judging from their own comments, and then not take responsibility for the 
breakage when that bug is fixed. However, I'll defer to @rsmith on whether this 
is sufficient reason for reverting, I do not feel qualified to make that 
judgement.

The MSVC compatibility, however, is something I hadn't foreseen and had just 
submitted a fix for, D95392 . If this change 
is to be reverted, I'll withdraw that followup fix for now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91913

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

[PATCH] D92808: [ObjC][ARC] Annotate calls with attributes instead of emitting retainRV or claimRV calls in the IR

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In D92808#2521042 , @rsmith wrote:

> This change violates layering by including an Analysis header from IR; I'll 
> be landing a revert as soon as I've finished testing it.

Should the header just be in IR?  We'd like to avoid duplicating constant 
strings unnecessarily between IRGen and IR passes.




Comment at: llvm/lib/IR/Instruction.cpp:580
+if (auto *CB = dyn_cast(this))
+  return objcarc::hasRetainRVOrClaimRVAttr(CB);
+return false;

nikic wrote:
> This change looks pretty fishy. Objective C shouldn't be hijacking LLVMs core 
> instruction model in this way. If it writes to memory, this should either be 
> reflected in the attributes, or modeled using operand bundles.
> 
> @fhahn Did you review these changes? If not, I'd suggest to revert this patch 
> and get a review on the LLVM changes.
This could definitely be an operand bundle, and I suppose the presence of a 
bundle does force a conservative assumption on passes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92808

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

[PATCH] D95392: Restore GNU , ## __VA_ARGS__ behavior in MSVC mode too

[Harald van Dijk via Phabricator via cfe-commits]

hvdijk created this revision.
hvdijk added a reviewer: rsmith.
hvdijk added a project: LLVM.
hvdijk requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

As noted in D91913 , MSVC implements the GNU 
behavior for , ## __VA_ARGS__ as well. Do the same when `-fms-compatibility` is 
used.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D95392

Files:
  clang/lib/Lex/TokenLexer.cpp
  clang/test/Preprocessor/macro_fn_comma_swallow2.c


Index: clang/test/Preprocessor/macro_fn_comma_swallow2.c
===
--- clang/test/Preprocessor/macro_fn_comma_swallow2.c
+++ clang/test/Preprocessor/macro_fn_comma_swallow2.c
@@ -9,6 +9,8 @@
 // RUN: %clang_cc1 -E -x c++ -std=c++11 %s | FileCheck -check-prefix=C99 
-strict-whitespace %s
 // RUN: %clang_cc1 -E -std=gnu99 %s | FileCheck -check-prefix=GCC 
-strict-whitespace %s
 // RUN: %clang_cc1 -E -fms-compatibility %s | FileCheck -check-prefix=MS 
-strict-whitespace %s
+// RUN: %clang_cc1 -E -x c++ -fms-compatibility %s | FileCheck 
-check-prefix=MS -strict-whitespace %s
+// RUN: %clang_cc1 -E -x c++ -std=c++11 -fms-compatibility %s | FileCheck 
-check-prefix=MS -strict-whitespace %s
 // RUN: %clang_cc1 -E -DNAMED %s | FileCheck -check-prefix=GCC 
-strict-whitespace %s
 // RUN: %clang_cc1 -E -std=c99 -DNAMED %s | FileCheck -check-prefix=C99 
-strict-whitespace %s
 
Index: clang/lib/Lex/TokenLexer.cpp
===
--- clang/lib/Lex/TokenLexer.cpp
+++ clang/lib/Lex/TokenLexer.cpp
@@ -152,7 +152,8 @@
   // named arguments, where it remains.  With GNU extensions, it is removed
   // regardless of named arguments.
   // Microsoft also appears to support this extension, unofficially.
-  if (!PP.getLangOpts().GNUMode && Macro->getNumParams() < 2)
+  if (!PP.getLangOpts().GNUMode && !PP.getLangOpts().MSVCCompat &&
+  Macro->getNumParams() < 2)
 return false;
 
   // Is a comma available to be removed?


Index: clang/test/Preprocessor/macro_fn_comma_swallow2.c
===
--- clang/test/Preprocessor/macro_fn_comma_swallow2.c
+++ clang/test/Preprocessor/macro_fn_comma_swallow2.c
@@ -9,6 +9,8 @@
 // RUN: %clang_cc1 -E -x c++ -std=c++11 %s | FileCheck -check-prefix=C99 -strict-whitespace %s
 // RUN: %clang_cc1 -E -std=gnu99 %s | FileCheck -check-prefix=GCC -strict-whitespace %s
 // RUN: %clang_cc1 -E -fms-compatibility %s | FileCheck -check-prefix=MS -strict-whitespace %s
+// RUN: %clang_cc1 -E -x c++ -fms-compatibility %s | FileCheck -check-prefix=MS -strict-whitespace %s
+// RUN: %clang_cc1 -E -x c++ -std=c++11 -fms-compatibility %s | FileCheck -check-prefix=MS -strict-whitespace %s
 // RUN: %clang_cc1 -E -DNAMED %s | FileCheck -check-prefix=GCC -strict-whitespace %s
 // RUN: %clang_cc1 -E -std=c99 -DNAMED %s | FileCheck -check-prefix=C99 -strict-whitespace %s
 
Index: clang/lib/Lex/TokenLexer.cpp
===
--- clang/lib/Lex/TokenLexer.cpp
+++ clang/lib/Lex/TokenLexer.cpp
@@ -152,7 +152,8 @@
   // named arguments, where it remains.  With GNU extensions, it is removed
   // regardless of named arguments.
   // Microsoft also appears to support this extension, unofficially.
-  if (!PP.getLangOpts().GNUMode && Macro->getNumParams() < 2)
+  if (!PP.getLangOpts().GNUMode && !PP.getLangOpts().MSVCCompat &&
+  Macro->getNumParams() < 2)
 return false;
 
   // Is a comma available to be removed?
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D94745: [OpenMP][WIP] Build the deviceRTLs with OpenMP instead of target dependent language - NOT FOR REVIEW

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 added inline comments.



Comment at: clang/lib/Driver/ToolChains/Cuda.cpp:785
+  std::string LibOmpTargetName = "libomptarget-nvptx-cuda_" +
+ CudaVersionStr + "-" + GpuArch.str() +
+ ".bc";

This change also requires changes in the driver tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94745

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

[PATCH] D94745: [OpenMP][WIP] Build the deviceRTLs with OpenMP instead of target dependent language - NOT FOR REVIEW

[Shilei Tian via Phabricator via cfe-commits]

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

Added the missing critical option `-fopenmp-cuda-mode`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94745

Files:
  clang/lib/Driver/ToolChains/Cuda.cpp
  openmp/libomptarget/deviceRTLs/common/allocator.h
  openmp/libomptarget/deviceRTLs/common/omptarget.h
  openmp/libomptarget/deviceRTLs/common/src/libcall.cu
  openmp/libomptarget/deviceRTLs/common/src/omp_data.cu
  openmp/libomptarget/deviceRTLs/common/src/reduction.cu
  openmp/libomptarget/deviceRTLs/nvptx/CMakeLists.txt
  openmp/libomptarget/deviceRTLs/nvptx/src/nvptx_interface.h
  openmp/libomptarget/deviceRTLs/nvptx/src/target_impl.cu
  openmp/libomptarget/deviceRTLs/nvptx/src/target_impl.h

Index: openmp/libomptarget/deviceRTLs/nvptx/src/target_impl.h
===
--- openmp/libomptarget/deviceRTLs/nvptx/src/target_impl.h
+++ openmp/libomptarget/deviceRTLs/nvptx/src/target_impl.h
@@ -13,18 +13,16 @@
 #define _TARGET_IMPL_H_
 
 #include 
-#include 
 #include 
 #include 
 #include 
 
 #include "nvptx_interface.h"
 
-#define DEVICE __device__
-#define INLINE __forceinline__ DEVICE
-#define NOINLINE __noinline__ DEVICE
-#define SHARED __shared__
-#define ALIGN(N) __align__(N)
+#define DEVICE
+#define INLINE inline __attribute__((always_inline))
+#define NOINLINE __attribute__((noinline))
+#define ALIGN(N) __attribute__((aligned(N)))
 
 
 // Kernel options
Index: openmp/libomptarget/deviceRTLs/nvptx/src/target_impl.cu
===
--- openmp/libomptarget/deviceRTLs/nvptx/src/target_impl.cu
+++ openmp/libomptarget/deviceRTLs/nvptx/src/target_impl.cu
@@ -14,8 +14,6 @@
 #include "target_impl.h"
 #include "common/debug.h"
 
-#include 
-
 DEVICE void __kmpc_impl_unpack(uint64_t val, uint32_t , uint32_t ) {
   asm volatile("mov.b64 {%0,%1}, %2;" : "=r"(lo), "=r"(hi) : "l"(val));
 }
Index: openmp/libomptarget/deviceRTLs/nvptx/src/nvptx_interface.h
===
--- openmp/libomptarget/deviceRTLs/nvptx/src/nvptx_interface.h
+++ openmp/libomptarget/deviceRTLs/nvptx/src/nvptx_interface.h
@@ -11,7 +11,8 @@
 
 #include 
 
-#define EXTERN extern "C" __device__
+#define EXTERN extern "C"
+
 typedef uint32_t __kmpc_impl_lanemask_t;
 typedef uint32_t omp_lock_t; /* arbitrary type of the right length */
 
Index: openmp/libomptarget/deviceRTLs/nvptx/CMakeLists.txt
===
--- openmp/libomptarget/deviceRTLs/nvptx/CMakeLists.txt
+++ openmp/libomptarget/deviceRTLs/nvptx/CMakeLists.txt
@@ -10,6 +10,21 @@
 #
 ##===--===##
 
+# TODO: This part needs to be refined when libomptarget is going to support
+# Windows!
+# TODO: This part can also be removed if we can change the clang driver to make
+# it support device only compilation.
+if(CMAKE_HOST_SYSTEM_PROCESSOR MATCHES "x86_64")
+  set(aux_triple x86_64-unknown-linux-gnu)
+elseif(CMAKE_HOST_SYSTEM_PROCESSOR MATCHES "ppc64le")
+  set(aux_triple powerpc64le-unknown-linux-gnu)
+elseif(CMAKE_HOST_SYSTEM_PROCESSOR MATCHES "aarch64")
+  set(aux_triple aarch64-unknown-linux-gnu)
+else()
+  libomptarget_say("Not building CUDA offloading device RTL: unknown host arch: ${CMAKE_HOST_SYSTEM_PROCESSOR}")
+  return()
+endif()
+
 get_filename_component(devicertl_base_directory
   ${CMAKE_CURRENT_SOURCE_DIR}
   DIRECTORY)
@@ -79,61 +94,82 @@
 )
 
 # Set flags for LLVM Bitcode compilation.
-set(bc_flags ${LIBOMPTARGET_NVPTX_SELECTED_CUDA_COMPILER_FLAGS}
+set(bc_flags -S -x c++
+ -target nvptx64
+ -Xclang -emit-llvm-bc
+ -Xclang -aux-triple -Xclang ${aux_triple}
+ -fopenmp -fopenmp-cuda-mode -Xclang -fopenmp-is-device
+ -D__CUDACC__
  -I${devicertl_base_directory}
  -I${devicertl_nvptx_directory}/src)
 
 if(${LIBOMPTARGET_NVPTX_DEBUG})
-  set(bc_flags ${bc_flags} -DOMPTARGET_NVPTX_DEBUG=-1)
+  list(APPEND bc_flags -DOMPTARGET_NVPTX_DEBUG=-1)
 else()
-  set(bc_flags ${bc_flags} -DOMPTARGET_NVPTX_DEBUG=0)
+  list(APPEND bc_flags -DOMPTARGET_NVPTX_DEBUG=0)
 endif()
 
 # Create target to build all Bitcode libraries.
 add_custom_target(omptarget-nvptx-bc)
 
-# Generate a Bitcode library for all the compute capabilities the user requested.
+# This correlation is from clang/lib/Driver/ToolChains/Cuda.cpp.
+# The last element is the default case.
+set(cuda_version_list 110 102 101 100 92 91 90 80)
+set(ptx_feature_list 70 65 64 63 61 61 60 42)
+
+# Generate a Bitcode library for all the compute capabilities the user
+# requested 

[PATCH] D92808: [ObjC][ARC] Annotate calls with attributes instead of emitting retainRV or claimRV calls in the IR

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

This change violates layering by including an Analysis header from IR; I'll be 
landing a revert as soon as I've finished testing it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92808

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

[PATCH] D94745: [OpenMP][WIP] Build the deviceRTLs with OpenMP instead of target dependent language - NOT FOR REVIEW

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield added inline comments.



Comment at: openmp/libomptarget/deviceRTLs/common/src/libcall.cu:319
 
-EXTERN int omp_is_initial_device(void) {
-  PRINT0(LD_IO, "call omp_is_initial_device() returns 0\n");
-  return 0; // 0 by def on device
-}
+// EXTERN int omp_is_initial_device(void) {
+//   PRINT0(LD_IO, "call omp_is_initial_device() returns 0\n");

tianshilei1992 wrote:
> I think we could safely delete this function because function call in device 
> code can always be taken as builtin so this function will never be called. 
> WDYT? @jdoerfert @JonChesterfield 
Yes. There is an existing bug that _is_initial_device doesn't work, but 
because of that nothing can call this function. We can reinstate it when 
replacing the builtin.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94745

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

[PATCH] D91913: Suppress non-conforming GNU paste extension in all standard-conforming modes

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

In D91913#2520503 , @hvdijk wrote:

> In D91913#2520432 , @zequanwu wrote:
>
>> In D91913#2520414 , @hvdijk wrote:
>>
>>> In D91913#2520399 , @zequanwu 
>>> wrote:
>>>
 This change also breaks many chrome ToT bots(not just windows bot), 
 example build: 
 https://ci.chromium.org/ui/p/chrome/builders/ci/ToTLinuxOfficial/10524/steps?succeeded=true=false
>>>
>>> That requires an account to see.
>>
>> This should be accessible without an account: 
>> https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket.appspot.com/8857139118288337920/+/steps/compile/0/stdout
>
> Indeed. However, here, this seems to be a user problem. Chrome `-std=c++*` 
> when building with clang, but `-std=gnu++*` when building with GCC for 
> exactly this reason, see build/config/compiler/BUILD.gn. Chrome should use 
> `-std=gnu++*` with clang too.

We (and every other project out there) needs some incremental rollout plan for 
this. We don't want to allow _more_ GNU features. Please put this behind a `-f` 
toggle, or revert this for a bit implementing this takes some time.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91913

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

[PATCH] D92715: [Clang][RISCV] Define RISC-V V builtin types

[Craig Topper via Phabricator via cfe-commits]

craig.topper added a comment.

I wonder if these types should be prefixed with "__clang_" like AArch64 tuple 
types?




Comment at: clang/lib/AST/ASTContext.cpp:3875
+IsFP)  
\
+  if (!EltTy->isBooleanType() &&   
\
+  ((EltTy->hasIntegerRepresentation() &&   
\

Should this be indented 2 more spaces? to align with the uin64_t above?



Comment at: clang/lib/AST/ASTContext.cpp:3882
+#define RVV_PREDICATE_TYPE(Name, Id, SingletonId, NumEls)  
\
+  if (EltTy->isBooleanType() && NumElts == NumEls) 
\
+return SingletonId;

Same here



Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:784
+  // Mask type only occupies one vector register.
+  if (Info.ElementType == CGM.getContext().BoolTy) {
+Fractional = false;

Put this above the "if (FixedSize < 64)" and make that an "else if" so all 
assignments are in the same if/else structure.
You could pre-initialize Fractional to false and only assign it to true in the 
if/else chain.



Comment at: clang/test/CodeGen/RISCV/riscv-v-debuginfo.c:1
+// RUN: %clang_cc1 -triple riscv64 -target-feature +experimental-v \
+// RUN:   -dwarf-version=4 -debug-info-kind=limited -emit-llvm -o - %s \

This seems to be dependent on the patch that adds the vadd builtins?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92715

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

[PATCH] D95307: [StaticAnalyzer] Add checking for degenerate base class in MemRegion

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

In D95307#2519597 , @vsavchenko wrote:

> In D95307#2519309 , @RedDocMD wrote:
>
>> Funnily enough, when I run `ninja clang-check` I don't get any errors.
>
> I believe that `ninja check-clang` is the right command (clang-check is a 
> tool) :-)

Inb4 `ninja check-clang-analysis` :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95307

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

[PATCH] D95307: [StaticAnalyzer] Add checking for degenerate base class in MemRegion

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

In D95307#2519283 , @vsavchenko wrote:

> In D95307#2518592 , @NoQ wrote:
>
>> This patch shoots the messenger but someone still needs to conduct a proper 
>> investigation. The assertion is losing a lot more of its bug-finding power 
>> than necessary to uncover the current cornercase; i really hope to preserve 
>> it instead.
>
> You mean the failing assertion from the tests?

I mean, `isValidBaseClass()` is a function that's only ever used as part of an 
assert condition; it has no other purpose. The only thing this patch does is 
partially disable the assertion. The patch also disables a lot more of the 
assertion than necessary for it to not fail on the test.

Disabling the assertion has relatively little value on its own because 
assertions are already disabled in release/production builds of clang. We need 
to understand whether it is the assertion that's incorrect, or it's the 
surrounding code that's incorrect. Given that we're performing a 
derived-to-base cast when such cast is not necessary, that's a good indication 
that the problem is in the surrounding code, i.e. the behavior implemented in 
the transfer function isn't a correct abstraction over the actual runtime 
behavior of the program under analysis. It might be hard to judge because typed 
pointer values are a pretty questionable abstraction to begin with but i think 
at least some investigation is justified.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95307

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

[PATCH] D94745: [OpenMP][WIP] Build the deviceRTLs with OpenMP instead of target dependent language - NOT FOR REVIEW

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 added inline comments.



Comment at: openmp/libomptarget/deviceRTLs/common/src/libcall.cu:319
 
-EXTERN int omp_is_initial_device(void) {
-  PRINT0(LD_IO, "call omp_is_initial_device() returns 0\n");
-  return 0; // 0 by def on device
-}
+// EXTERN int omp_is_initial_device(void) {
+//   PRINT0(LD_IO, "call omp_is_initial_device() returns 0\n");

I think we could safely delete this function because function call in device 
code can always be taken as builtin so this function will never be called. 
WDYT? @jdoerfert @JonChesterfield 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94745

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

[PATCH] D92808: [ObjC][ARC] Annotate calls with attributes instead of emitting retainRV or claimRV calls in the IR

[Nikita Popov via Phabricator via cfe-commits]

nikic added inline comments.



Comment at: llvm/lib/IR/Instruction.cpp:580
+if (auto *CB = dyn_cast(this))
+  return objcarc::hasRetainRVOrClaimRVAttr(CB);
+return false;

This change looks pretty fishy. Objective C shouldn't be hijacking LLVMs core 
instruction model in this way. If it writes to memory, this should either be 
reflected in the attributes, or modeled using operand bundles.

@fhahn Did you review these changes? If not, I'd suggest to revert this patch 
and get a review on the LLVM changes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92808

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

[PATCH] D94745: [OpenMP][WIP] Build the deviceRTLs with OpenMP instead of target dependent language - NOT FOR REVIEW

[Shilei Tian via Phabricator via cfe-commits]

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

rebased and dropped cuda header


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94745

Files:
  clang/lib/Driver/ToolChains/Cuda.cpp
  openmp/libomptarget/deviceRTLs/common/allocator.h
  openmp/libomptarget/deviceRTLs/common/omptarget.h
  openmp/libomptarget/deviceRTLs/common/src/libcall.cu
  openmp/libomptarget/deviceRTLs/common/src/omp_data.cu
  openmp/libomptarget/deviceRTLs/common/src/reduction.cu
  openmp/libomptarget/deviceRTLs/nvptx/CMakeLists.txt
  openmp/libomptarget/deviceRTLs/nvptx/src/nvptx_interface.h
  openmp/libomptarget/deviceRTLs/nvptx/src/target_impl.cu
  openmp/libomptarget/deviceRTLs/nvptx/src/target_impl.h

Index: openmp/libomptarget/deviceRTLs/nvptx/src/target_impl.h
===
--- openmp/libomptarget/deviceRTLs/nvptx/src/target_impl.h
+++ openmp/libomptarget/deviceRTLs/nvptx/src/target_impl.h
@@ -13,18 +13,16 @@
 #define _TARGET_IMPL_H_
 
 #include 
-#include 
 #include 
 #include 
 #include 
 
 #include "nvptx_interface.h"
 
-#define DEVICE __device__
-#define INLINE __forceinline__ DEVICE
-#define NOINLINE __noinline__ DEVICE
-#define SHARED __shared__
-#define ALIGN(N) __align__(N)
+#define DEVICE
+#define INLINE inline __attribute__((always_inline))
+#define NOINLINE __attribute__((noinline))
+#define ALIGN(N) __attribute__((aligned(N)))
 
 
 // Kernel options
Index: openmp/libomptarget/deviceRTLs/nvptx/src/target_impl.cu
===
--- openmp/libomptarget/deviceRTLs/nvptx/src/target_impl.cu
+++ openmp/libomptarget/deviceRTLs/nvptx/src/target_impl.cu
@@ -14,8 +14,6 @@
 #include "target_impl.h"
 #include "common/debug.h"
 
-#include 
-
 DEVICE void __kmpc_impl_unpack(uint64_t val, uint32_t , uint32_t ) {
   asm volatile("mov.b64 {%0,%1}, %2;" : "=r"(lo), "=r"(hi) : "l"(val));
 }
Index: openmp/libomptarget/deviceRTLs/nvptx/src/nvptx_interface.h
===
--- openmp/libomptarget/deviceRTLs/nvptx/src/nvptx_interface.h
+++ openmp/libomptarget/deviceRTLs/nvptx/src/nvptx_interface.h
@@ -11,7 +11,8 @@
 
 #include 
 
-#define EXTERN extern "C" __device__
+#define EXTERN extern "C"
+
 typedef uint32_t __kmpc_impl_lanemask_t;
 typedef uint32_t omp_lock_t; /* arbitrary type of the right length */
 
Index: openmp/libomptarget/deviceRTLs/nvptx/CMakeLists.txt
===
--- openmp/libomptarget/deviceRTLs/nvptx/CMakeLists.txt
+++ openmp/libomptarget/deviceRTLs/nvptx/CMakeLists.txt
@@ -10,6 +10,21 @@
 #
 ##===--===##
 
+# TODO: This part needs to be refined when libomptarget is going to support
+# Windows!
+# TODO: This part can also be removed if we can change the clang driver to make
+# it support device only compilation.
+if(CMAKE_HOST_SYSTEM_PROCESSOR MATCHES "x86_64")
+  set(aux_triple x86_64-unknown-linux-gnu)
+elseif(CMAKE_HOST_SYSTEM_PROCESSOR MATCHES "ppc64le")
+  set(aux_triple powerpc64le-unknown-linux-gnu)
+elseif(CMAKE_HOST_SYSTEM_PROCESSOR MATCHES "aarch64")
+  set(aux_triple aarch64-unknown-linux-gnu)
+else()
+  libomptarget_say("Not building CUDA offloading device RTL: unknown host arch: ${CMAKE_HOST_SYSTEM_PROCESSOR}")
+  return()
+endif()
+
 get_filename_component(devicertl_base_directory
   ${CMAKE_CURRENT_SOURCE_DIR}
   DIRECTORY)
@@ -79,61 +94,82 @@
 )
 
 # Set flags for LLVM Bitcode compilation.
-set(bc_flags ${LIBOMPTARGET_NVPTX_SELECTED_CUDA_COMPILER_FLAGS}
+set(bc_flags -S -x c++
+ -target nvptx64
+ -Xclang -emit-llvm-bc
+ -Xclang -aux-triple -Xclang ${aux_triple}
+ -fopenmp -Xclang -fopenmp-is-device
+ -D__CUDACC__
  -I${devicertl_base_directory}
  -I${devicertl_nvptx_directory}/src)
 
 if(${LIBOMPTARGET_NVPTX_DEBUG})
-  set(bc_flags ${bc_flags} -DOMPTARGET_NVPTX_DEBUG=-1)
+  list(APPEND bc_flags -DOMPTARGET_NVPTX_DEBUG=-1)
 else()
-  set(bc_flags ${bc_flags} -DOMPTARGET_NVPTX_DEBUG=0)
+  list(APPEND bc_flags -DOMPTARGET_NVPTX_DEBUG=0)
 endif()
 
 # Create target to build all Bitcode libraries.
 add_custom_target(omptarget-nvptx-bc)
 
-# Generate a Bitcode library for all the compute capabilities the user requested.
+# This correlation is from clang/lib/Driver/ToolChains/Cuda.cpp.
+# The last element is the default case.
+set(cuda_version_list 110 102 101 100 92 91 90 80)
+set(ptx_feature_list 70 65 64 63 61 61 60 42)
+
+# Generate a Bitcode library for all the compute capabilities the user
+# requested and all PTX version we know for now.
 

[PATCH] D91054: [Clang][OpenMP] Frontend work for sections - D89671

[Chirag Khandelwal via Phabricator via cfe-commits]

AMDChirag updated this revision to Diff 319095.
AMDChirag added a comment.

Added FIXME comment for cancellation construct not working with sections 
construct
Also updated the lit test cases to reflect this change.
The lit test case modification will be removed once this issue is resolved.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91054

Files:
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/test/OpenMP/cancel_codegen.cpp
  clang/test/OpenMP/cancel_codegen_noirbuilder.cpp
  clang/test/OpenMP/cancel_sections_irbuilder.cpp

Index: clang/test/OpenMP/cancel_sections_irbuilder.cpp
===
--- /dev/null
+++ clang/test/OpenMP/cancel_sections_irbuilder.cpp
@@ -0,0 +1,26 @@
+// RUN: %clang_cc1 -verify -fopenmp -fopenmp-version=45 -fopenmp-enable-irbuilder -triple x86_64-apple-darwin13.4.0 -emit-llvm -o - %s | FileCheck %s --check-prefixes=ALL,IRBUILDER
+// RUN: %clang_cc1 -fopenmp -fopenmp-version=45 -fopenmp-enable-irbuilder -x c++ -std=c++11 -triple x86_64-apple-darwin13.4.0 -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp -fopenmp-version=45 -fopenmp-enable-irbuilder -std=c++11 -include-pch %t -fsyntax-only -verify %s -triple x86_64-apple-darwin13.4.0 -emit-llvm -o - | FileCheck %s --check-prefixes=ALL,IRBUILDER
+
+// RUN: %clang_cc1 -verify -fopenmp -fopenmp-enable-irbuilder -triple x86_64-apple-darwin13.4.0 -emit-llvm -o - %s | FileCheck %s --check-prefixes=ALL,IRBUILDER
+// RUN: %clang_cc1 -fopenmp -fopenmp-enable-irbuilder -x c++ -std=c++11 -triple x86_64-apple-darwin13.4.0 -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp -fopenmp-enable-irbuilder -std=c++11 -include-pch %t -fsyntax-only -verify %s -triple x86_64-apple-darwin13.4.0 -emit-llvm -o - | FileCheck %s --check-prefixes=ALL,IRBUILDER
+
+// XFAIL: *
+
+// FIXME: Sections construct with cancellation directive currently fails,
+// because EmitOMPRegionBody() removes any existing block terminator
+// which in turn causes FiniCB callback to fail in FinalizeOMPRegion()
+// as no terminator instruction (br) exists for the cancel block (.cncl)
+
+int main(int argc, char **argv) {
+#pragma omp sections
+  {
+#pragma omp section
+{
+#pragma omp cancel sections
+  argv[0][0] = argc;
+}
+  }
+  return 0;
+}
Index: clang/test/OpenMP/cancel_codegen_noirbuilder.cpp
===
--- clang/test/OpenMP/cancel_codegen_noirbuilder.cpp
+++ clang/test/OpenMP/cancel_codegen_noirbuilder.cpp
@@ -2,10 +2,6 @@
 // RUN: %clang_cc1 -fopenmp -fopenmp-version=45 -x c++ -std=c++11 -triple x86_64-apple-darwin13.4.0 -emit-pch -o %t %s
 // RUN: %clang_cc1 -fopenmp -fopenmp-version=45 -std=c++11 -include-pch %t -fsyntax-only -verify %s -triple x86_64-apple-darwin13.4.0 -emit-llvm -o - | FileCheck %s --check-prefixes=ALL,CHECK
 
-// RUN: %clang_cc1 -verify -fopenmp -fopenmp-version=45 -fopenmp-enable-irbuilder -triple x86_64-apple-darwin13.4.0 -emit-llvm -o - %s | FileCheck %s --check-prefixes=ALL,IRBUILDER
-// RUN: %clang_cc1 -fopenmp -fopenmp-version=45 -fopenmp-enable-irbuilder -x c++ -std=c++11 -triple x86_64-apple-darwin13.4.0 -emit-pch -o %t %s
-// RUN: %clang_cc1 -fopenmp -fopenmp-version=45 -fopenmp-enable-irbuilder -std=c++11 -include-pch %t -fsyntax-only -verify %s -triple x86_64-apple-darwin13.4.0 -emit-llvm -o - | FileCheck %s --check-prefixes=ALL,IRBUILDER
-
 // RUN: %clang_cc1 -verify -fopenmp-simd -fopenmp-version=45 -triple x86_64-apple-darwin13.4.0 -emit-llvm -o - %s | FileCheck --check-prefix SIMD-ONLY0 %s
 // RUN: %clang_cc1 -fopenmp-simd -fopenmp-version=45 -x c++ -std=c++11 -triple x86_64-apple-darwin13.4.0 -emit-pch -o %t %s
 // RUN: %clang_cc1 -fopenmp-simd -fopenmp-version=45 -std=c++11 -include-pch %t -fsyntax-only -verify %s -triple x86_64-apple-darwin13.4.0 -emit-llvm -o - | FileCheck --check-prefix SIMD-ONLY0 %s
@@ -15,45 +11,43 @@
 // RUN: %clang_cc1 -fopenmp -x c++ -std=c++11 -triple x86_64-apple-darwin13.4.0 -emit-pch -o %t %s
 // RUN: %clang_cc1 -fopenmp -std=c++11 -include-pch %t -fsyntax-only -verify %s -triple x86_64-apple-darwin13.4.0 -emit-llvm -o - | FileCheck %s --check-prefixes=ALL,CHECK
 
-// RUN: %clang_cc1 -verify -fopenmp -fopenmp-enable-irbuilder -triple x86_64-apple-darwin13.4.0 -emit-llvm -o - %s | FileCheck %s --check-prefixes=ALL,IRBUILDER
-// RUN: %clang_cc1 -fopenmp -fopenmp-enable-irbuilder -x c++ -std=c++11 -triple x86_64-apple-darwin13.4.0 -emit-pch -o %t %s
-// RUN: %clang_cc1 -fopenmp -fopenmp-enable-irbuilder -std=c++11 -include-pch %t -fsyntax-only -verify %s -triple x86_64-apple-darwin13.4.0 -emit-llvm -o - | FileCheck %s --check-prefixes=ALL,IRBUILDER
-
 // RUN: %clang_cc1 -verify -fopenmp-simd -triple x86_64-apple-darwin13.4.0 -emit-llvm -o - %s | FileCheck --check-prefix SIMD-ONLY0 %s
 // RUN: %clang_cc1 -fopenmp-simd -x c++ -std=c++11 -triple x86_64-apple-darwin13.4.0 -emit-pch -o %t %s
 // RUN: %clang_cc1 

[PATCH] D95385: Revert "[clangd][NFC] Simplify handing on methods with no params"

[Keith Smiley via Phabricator via cfe-commits]

keith added a comment.

No problem, thanks!!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95385

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

[PATCH] D94820: Support for instrumenting only selected files or functions

[David Li via Phabricator via cfe-commits]

davidxl added inline comments.



Comment at: clang/test/CodeGen/profile-filter.c:17
+// RUN: %clang_cc1 -fprofile-instrument=clang 
-fprofile-list=%t-exclude-only.list -emit-llvm %s -o - | FileCheck %s 
--check-prefix=EXCLUDE
+
+unsigned i;

Can you add a test case with both include and exclude list (the intersection of 
INCLUDE_SET & !EXCLUDE_SET)?


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

https://reviews.llvm.org/D94820

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

[PATCH] D95385: Revert "[clangd][NFC] Simplify handing on methods with no params"

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

Thanks for reverting this, That old full specialization allowed at class scope 
defect report always gets me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95385

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

[PATCH] D94803: [clang][cli] Generate HeaderSearch options separately

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

dexonsmith accepted this revision.
dexonsmith 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/D94803/new/

https://reviews.llvm.org/D94803

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

[PATCH] D94802: [clang][cli] Parse HeaderSearch options separately

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

dexonsmith accepted this revision.
dexonsmith 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/D94802/new/

https://reviews.llvm.org/D94802

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

[PATCH] D95343: [clang][cli] Port GNU language options to marshalling system

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

dexonsmith accepted this revision.
dexonsmith 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/D95343/new/

https://reviews.llvm.org/D95343

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

[PATCH] D95249: [NFC] Disallow unused prefixes in clang/test/Analysis

[Artem Dergachev via Phabricator via cfe-commits]

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

Thanks!!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95249

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

[PATCH] D95347: [clang][cli] Port LangOpts to marshalling system, pt. 2

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

dexonsmith accepted this revision.
dexonsmith 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/D95347/new/

https://reviews.llvm.org/D95347

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

[clang] f02eca0 - [clang-format] [NFC] Rerun dump_format_style.py

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

Author: Björn Schäpers
Date: 2021-01-25T21:02:41+01:00
New Revision: f02eca0f3feffc5e1afb92eeb4cfc23b9f28aa25

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

LOG: [clang-format] [NFC] Rerun dump_format_style.py

Added: 


Modified: 
clang/docs/ClangFormatStyleOptions.rst

Removed: 




diff  --git a/clang/docs/ClangFormatStyleOptions.rst 
b/clang/docs/ClangFormatStyleOptions.rst
index f6ff57156177..c12ecf4d907f 100644
--- a/clang/docs/ClangFormatStyleOptions.rst
+++ b/clang/docs/ClangFormatStyleOptions.rst
@@ -2360,14 +2360,15 @@ the configuration (without a prefix: ``Auto``).
   When ``true``, includes are sorted in an alphabetical
   fashion with case used as a tie-breaker.
 
+
   .. code-block:: c++
 
- false:   true:
- #include "A/B.h"   vs.   #include "A/B.h"
- #include "A/b.h" #include "A/b.h"
- #include "B/A.h" #include "a/b.h"
- #include "B/a.h" #include "B/A.h"
- #include "a/b.h" #include "B/a.h"
+false:   true:
+#include "A/B.h"   vs.   #include "A/B.h"
+#include "A/b.h" #include "A/b.h"
+#include "B/A.h" #include "a/b.h"
+#include "B/a.h" #include "B/A.h"
+#include "a/b.h" #include "B/a.h"
 
   This option is off by default.
 



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

[PATCH] D95342: [clang][cli] Store LangStandard::Kind in LangOptions

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

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

LGTM.




Comment at: clang/include/clang/Basic/LangOptions.h:18
 #include "clang/Basic/CommentOptions.h"
+#include "clang/Basic/LangStandard.h"
 #include "clang/Basic/LLVM.h"

Nit: clang-format wants you to move this for better `#include` sorting.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95342

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

[PATCH] D93846: [clang-format] PR16518 Add flag to suppress empty line insertion before access modifier

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rG60bf5826cfd3: [clang-format] PR16518 Add flag to suppress 
empty line insertion before access… (authored by thezbyg, committed by 
HazardyKnusperkeks).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93846

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/UnwrappedLineFormatter.cpp
  clang/test/Format/access-modifiers.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -8887,6 +8887,298 @@
getLLVMStyle());
 }
 
+TEST_F(FormatTest, FormatsAccessModifiers) {
+  FormatStyle Style = getLLVMStyle();
+  EXPECT_EQ(Style.EmptyLineBeforeAccessModifier,
+FormatStyle::ELBAMS_LogicalBlock);
+  verifyFormat("struct foo {\n"
+   "private:\n"
+   "  void f() {}\n"
+   "\n"
+   "private:\n"
+   "  int i;\n"
+   "\n"
+   "protected:\n"
+   "  int j;\n"
+   "};\n",
+   Style);
+  verifyFormat("struct foo {\n"
+   "private:\n"
+   "  void f() {}\n"
+   "\n"
+   "private:\n"
+   "  int i;\n"
+   "\n"
+   "protected:\n"
+   "  int j;\n"
+   "};\n",
+   "struct foo {\n"
+   "private:\n"
+   "  void f() {}\n"
+   "private:\n"
+   "  int i;\n"
+   "protected:\n"
+   "  int j;\n"
+   "};\n",
+   Style);
+  verifyFormat("struct foo { /* comment */\n"
+   "private:\n"
+   "  int i;\n"
+   "  // comment\n"
+   "private:\n"
+   "  int j;\n"
+   "};\n",
+   Style);
+  verifyFormat("struct foo {\n"
+   "#ifdef FOO\n"
+   "#endif\n"
+   "private:\n"
+   "  int i;\n"
+   "#ifdef FOO\n"
+   "private:\n"
+   "#endif\n"
+   "  int j;\n"
+   "};\n",
+   Style);
+  Style.EmptyLineBeforeAccessModifier = FormatStyle::ELBAMS_Never;
+  verifyFormat("struct foo {\n"
+   "private:\n"
+   "  void f() {}\n"
+   "private:\n"
+   "  int i;\n"
+   "protected:\n"
+   "  int j;\n"
+   "};\n",
+   Style);
+  verifyFormat("struct foo {\n"
+   "private:\n"
+   "  void f() {}\n"
+   "private:\n"
+   "  int i;\n"
+   "protected:\n"
+   "  int j;\n"
+   "};\n",
+   "struct foo {\n"
+   "\n"
+   "private:\n"
+   "  void f() {}\n"
+   "\n"
+   "private:\n"
+   "  int i;\n"
+   "\n"
+   "protected:\n"
+   "  int j;\n"
+   "};\n",
+   Style);
+  verifyFormat("struct foo { /* comment */\n"
+   "private:\n"
+   "  int i;\n"
+   "  // comment\n"
+   "private:\n"
+   "  int j;\n"
+   "};\n",
+   "struct foo { /* comment */\n"
+   "\n"
+   "private:\n"
+   "  int i;\n"
+   "  // comment\n"
+   "\n"
+   "private:\n"
+   "  int j;\n"
+   "};\n",
+   Style);
+  verifyFormat("struct foo {\n"
+   "#ifdef FOO\n"
+   "#endif\n"
+   "private:\n"
+   "  int i;\n"
+   "#ifdef FOO\n"
+   "private:\n"
+   "#endif\n"
+   "  int j;\n"
+   "};\n",
+   "struct foo {\n"
+   "#ifdef FOO\n"
+   "#endif\n"
+   "\n"
+   "private:\n"
+   "  int i;\n"
+   "#ifdef FOO\n"
+   "\n"
+   "private:\n"
+   "#endif\n"
+   "  int j;\n"
+   "};\n",
+   Style);
+  Style.EmptyLineBeforeAccessModifier = FormatStyle::ELBAMS_Always;
+  verifyFormat("struct foo {\n"
+   "private:\n"
+   "  void f() {}\n"
+   "\n"
+   "private:\n"
+   "  int i;\n"
+   "\n"
+   "protected:\n"
+   "  int j;\n"
+   "};\n",
+   Style);
+  verifyFormat("struct foo {\n"
+   "private:\n"
+   "  void f() {}\n"
+ 

[clang] 60bf582 - [clang-format] PR16518 Add flag to suppress empty line insertion before access modifier

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

Author: Albertas Vyšniauskas
Date: 2021-01-25T21:02:41+01:00
New Revision: 60bf5826cfd3629b5200a8ab743d701c90f66af0

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

LOG: [clang-format] PR16518 Add flag to suppress empty line insertion before 
access modifier

Add new option called InsertEmptyLineBeforeAccessModifier. Empty line
before access modifier is inerted if this option is set to true (which
is the default value, because clang-format always inserts empty lines
before access modifiers), otherwise empty lines are removed.

Fixes issue #16518.

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

Added: 
clang/test/Format/access-modifiers.cpp

Modified: 
clang/docs/ClangFormatStyleOptions.rst
clang/include/clang/Format/Format.h
clang/lib/Format/Format.cpp
clang/lib/Format/UnwrappedLineFormatter.cpp
clang/unittests/Format/FormatTest.cpp

Removed: 




diff  --git a/clang/docs/ClangFormatStyleOptions.rst 
b/clang/docs/ClangFormatStyleOptions.rst
index d5ce1b7b2e8e..f6ff57156177 100644
--- a/clang/docs/ClangFormatStyleOptions.rst
+++ b/clang/docs/ClangFormatStyleOptions.rst
@@ -2119,6 +2119,75 @@ the configuration (without a prefix: ``Auto``).
 **DisableFormat** (``bool``)
   Disables formatting completely.
 
+**EmptyLineBeforeAccessModifier** (``EmptyLineBeforeAccessModifierStyle``)
+  Defines in which cases to put empty line before access modifiers.
+
+  Possible values:
+
+  * ``ELBAMS_Never`` (in configuration: ``Never``)
+Remove all empty lines before access modifiers.
+
+.. code-block:: c++
+
+  struct foo {
+  private:
+int i;
+  protected:
+int j;
+/* comment */
+  public:
+foo() {}
+  private:
+  protected:
+  };
+
+  * ``ELBAMS_Leave`` (in configuration: ``Leave``)
+Keep existing empty lines before access modifiers.
+
+  * ``ELBAMS_LogicalBlock`` (in configuration: ``LogicalBlock``)
+Add empty line only when access modifier starts a new logical block.
+Logical block is a group of one or more member fields or functions.
+
+.. code-block:: c++
+
+  struct foo {
+  private:
+int i;
+
+  protected:
+int j;
+/* comment */
+  public:
+foo() {}
+
+  private:
+  protected:
+  };
+
+  * ``ELBAMS_Always`` (in configuration: ``Always``)
+Always add empty line before access modifiers unless access modifier
+is at the start of struct or class definition.
+
+.. code-block:: c++
+
+  struct foo {
+  private:
+int i;
+
+  protected:
+int j;
+/* comment */
+
+  public:
+foo() {}
+
+  private:
+
+  protected:
+  };
+
+
+
 **ExperimentalAutoDetectBinPacking** (``bool``)
   If ``true``, clang-format detects whether function calls and
   definitions are formatted with one parameter per line.

diff  --git a/clang/include/clang/Format/Format.h 
b/clang/include/clang/Format/Format.h
index fcc38e25542f..96c2a74e97db 100644
--- a/clang/include/clang/Format/Format.h
+++ b/clang/include/clang/Format/Format.h
@@ -1885,6 +1885,68 @@ struct FormatStyle {
   /// Disables formatting completely.
   bool DisableFormat;
 
+  /// Different styles for empty line before access modifiers.
+  enum EmptyLineBeforeAccessModifierStyle : unsigned char {
+/// Remove all empty lines before access modifiers.
+/// \code
+///   struct foo {
+///   private:
+/// int i;
+///   protected:
+/// int j;
+/// /* comment */
+///   public:
+/// foo() {}
+///   private:
+///   protected:
+///   };
+/// \endcode
+ELBAMS_Never,
+/// Keep existing empty lines before access modifiers.
+ELBAMS_Leave,
+/// Add empty line only when access modifier starts a new logical block.
+/// Logical block is a group of one or more member fields or functions.
+/// \code
+///   struct foo {
+///   private:
+/// int i;
+///
+///   protected:
+/// int j;
+/// /* comment */
+///   public:
+/// foo() {}
+///
+///   private:
+///   protected:
+///   };
+/// \endcode
+ELBAMS_LogicalBlock,
+/// Always add empty line before access modifiers unless access modifier
+/// is at the start of struct or class definition.
+/// \code
+///   struct foo {
+///   private:
+/// int i;
+///
+///   protected:
+/// int j;
+/// /* comment */
+///
+///   public:
+/// foo() {}
+///
+///   private:
+///
+///   protected:
+///   };
+/// \endcode
+ELBAMS_Always,
+  };
+
+  /// Defines in which cases to put empty line before access modifiers.
+  EmptyLineBeforeAccessModifierStyle