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

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak created this revision.
ahatanak added reviewers: rjmccall, smeenai, dexonsmith, pete, fhahn.
ahatanak added projects: clang, LLVM.
Herald added subscribers: ributzka, pengfei, jkorous, hiraditya, kristof.beyls.
ahatanak requested review of this revision.

Background:
===

This fixes a longstanding problem where llvm breaks ARC's autorelease 
optimization (see the link below) by separating calls from the marker 
instructions or retainRV/claimRV calls.

https://clang.llvm.org/docs/AutomaticReferenceCounting.html#arc-runtime-objc-autoreleasereturnvalue

What this patch does to fix the problem:


- The front-end annotates calls with attribute retainRV/claimRV, which 
indicates the call result is implicitly consumed by a retainRV/claimRV call, 
and attribute rv_marker, which indicates a marker instruction should 
immediately follow the call. This is currently done only when the target is 
arm64, the optimization level is higher than -O0, and the OS isn't windows.

- ARC optimizer temporarily emits retainRV/claimRV calls in the IR if it finds 
a call annotated with the attributes and removes the inserted calls after 
processing the function.

- ARC contract pass emits the retainRV/claimRV calls in the IR and removes the 
retainRV/claimRV attribute from the call, but leaves the rv_marker attribute on 
the call. This is done late in the pipeline to prevent optimization passes from 
transforming the IR in a way that makes it harder for the ARC middle-end passes 
to figure out the def-use relationship between the call and the 
retainRV/claimRV calls (which is the cause of PR31925).

- The backend emits a pseudo instruction when it sees a call annotated with 
rv_marker and expands it later in the pipeline to a bundled instruction (see 
https://reviews.llvm.org/D92569).

Future work:


- Use the attribute on x86-64.

- Use the attribute on windows too.

- Fix the auto upgrader to convert call+retainRV/claimRV pairs into calls 
annotated with the attributes.

rdar://problem/32902328


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D92808

Files:
  clang/lib/CodeGen/CGObjC.cpp
  clang/test/CodeGenObjC/arc-rv-attr.m
  clang/test/CodeGenObjC/arc-unsafeclaim.m
  llvm/include/llvm/Analysis/ObjCARCRVAttr.h
  llvm/include/llvm/IR/InstrTypes.h
  llvm/lib/IR/Instruction.cpp
  llvm/lib/IR/Instructions.cpp
  llvm/lib/Transforms/IPO/DeadArgumentElimination.cpp
  llvm/lib/Transforms/ObjCARC/ARCRuntimeEntryPoints.h
  llvm/lib/Transforms/ObjCARC/ObjCARC.cpp
  llvm/lib/Transforms/ObjCARC/ObjCARC.h
  llvm/lib/Transforms/ObjCARC/ObjCARCContract.cpp
  llvm/lib/Transforms/ObjCARC/ObjCARCOpts.cpp
  llvm/lib/Transforms/ObjCARC/PtrState.cpp
  llvm/lib/Transforms/ObjCARC/PtrState.h
  llvm/lib/Transforms/Utils/InlineFunction.cpp
  llvm/test/Transforms/DeadArgElim/deadretval.ll
  llvm/test/Transforms/Inline/inline-retainRV-call.ll
  llvm/test/Transforms/ObjCARC/contract-rv-attr.ll
  llvm/test/Transforms/ObjCARC/rv.ll

Index: llvm/test/Transforms/ObjCARC/rv.ll
===
--- llvm/test/Transforms/ObjCARC/rv.ll
+++ llvm/test/Transforms/ObjCARC/rv.ll
@@ -452,6 +452,28 @@
   ret i8* %v3
 }
 
+; Remove attributes and the autoreleaseRV call if the call is a tail call.
+
+; CHECK-LABEL: define i8* @test31(
+; CHECK: %[[CALL:.*]] = tail call i8* @returner()
+; CHECK: ret i8* %[[CALL]]
+
+define i8* @test31() {
+  %call = tail call "retainRV" "rv_marker" i8* @returner()
+  %1 = call i8* @llvm.objc.autoreleaseReturnValue(i8* %call)
+  ret i8* %1
+}
+
+; CHECK-LABEL: define i8* @test32(
+; CHECK: %[[CALL:.*]] = call "retainRV" "rv_marker" i8* @returner()
+; CHECK: call i8* @llvm.objc.autoreleaseReturnValue(i8* %[[CALL]])
+
+define i8* @test32() {
+  %call = call "retainRV" "rv_marker" i8* @returner()
+  %1 = call i8* @llvm.objc.autoreleaseReturnValue(i8* %call)
+  ret i8* %1
+}
+
 !0 = !{}
 
 ; CHECK: attributes [[NUW]] = { nounwind }
Index: llvm/test/Transforms/ObjCARC/contract-rv-attr.ll
===
--- /dev/null
+++ llvm/test/Transforms/ObjCARC/contract-rv-attr.ll
@@ -0,0 +1,63 @@
+; RUN: opt -objc-arc-contract -S < %s | FileCheck %s
+; RUN: opt -passes=objc-arc-contract -S < %s | FileCheck %s
+
+; CHECK-LABEL: define void @test0() {
+; CHECK: %[[CALL:.*]] = notail call "rv_marker" i8* @foo()
+; CHECK: call i8* @llvm.objc.retainAutoreleasedReturnValue(i8* %[[CALL]])
+
+define void @test0() {
+  %call1 = call "retainRV" "rv_marker" i8* @foo()
+  ret void
+}
+
+; CHECK-LABEL: define void @test1() {
+; CHECK: %[[CALL:.*]] = notail call "rv_marker" i8* @foo()
+; CHECK: call i8* @llvm.objc.unsafeClaimAutoreleasedReturnValue(i8* %[[CALL]])
+
+define void @test1() {
+  %call1 = call "claimRV" "rv_marker" i8* @foo()
+  ret void
+}
+
+; CHECK-LABEL:define i8* @test2(
+; CHECK: %[[CALL1:.*]] = invoke "rv_marker" i8* @foo()
+
+; CHECK: %[[V0:.*]] = call i8* @llvm.objc.

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

[Shoaib Meenai via Phabricator via cfe-commits]

smeenai added a reviewer: compnerd.
smeenai resigned from this revision.
smeenai added a subscriber: compnerd.
smeenai added a comment.

I'm super excited to see this happen, but it's also well out of my review 
comfort zone, unfortunately. Adding @compnerd to take a look.


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

[Zoe Carver via Phabricator via cfe-commits]

zoecarver added a comment.

In D87974#2438682 , @BillyONeal wrote:

>> Are they actually the same, with the same handling of corner cases like 
>> unions and tail padding?
>> There's more to this than just the name, and if they aren't the same, it 
>> seems better to have two names.
>
> They are both implementing the same C++ feature, with the same desired 
> semantics of zeroing out any bits in the object representation that aren't in 
> the value representation. If they differ, one or the other would have a bug.

I agree, they either need to be identical (including corner cases) or there 
needs to be two of them (i.e., GCC ships both `__builtin_zero_non_value_bits` 
and `__builtin_clear_padding` and the first is the same as MSVC, Clang, and 
NVCC).

>> Is there a specification for __builtin_zero_non_value_bits available 
>> somewhere?
>
> I don't know if there is a formal spec for it beyond the actual C++ standard.

I think P0528 is the relevant paper but other than that, no, there's not a 
spec. I think that's going to be the most time sensitive part of implementing 
this: coming up with the spec and making sure all the tests pass on all the 
implementations.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87974

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

[PATCH] D92809: [Driver] Add -gno-split-dwarf which can disable debug fission

[Fangrui Song via Phabricator via cfe-commits]

MaskRay created this revision.
MaskRay added reviewers: debug-info, dblaikie.
Herald added a subscriber: dang.
MaskRay requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Currently when -gsplit-dwarf is specified (could be buried in a build system),
there is no convenient way to cancel debug fission without affecting the debug
information amount (all of -g0, -g1 -fsplit-dwarf-inlining, and 
-gline-directives-only
can, but they affect the debug information amount).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D92809

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/split-debug.c


Index: clang/test/Driver/split-debug.c
===
--- clang/test/Driver/split-debug.c
+++ clang/test/Driver/split-debug.c
@@ -17,12 +17,21 @@
 // RUN: %clang -### -c -target amdgcn-amd-amdhsa -gsplit-dwarf -g %s 2>&1 | 
FileCheck %s --check-prefix=SPLIT
 
 /// -gsplit-dwarf is a no-op on a non-ELF platform.
-// RUN: %clang -### -c -target x86_64-apple-darwin  -gsplit-dwarf -g %s 2>&1 | 
FileCheck %s --check-prefix=NOSPLIT
-// NOSPLIT-NOT: "-split-dwarf
+// RUN: %clang -### -c -target x86_64-apple-darwin  -gsplit-dwarf -g %s 2>&1 | 
FileCheck %s --check-prefix=DARWIN
+// DARWIN: "-debug-info-kind=standalone"
+// DARWIN-NOT: "-split-dwarf
 
 /// -gsplit-dwarf is a no-op if no -g is specified.
 // RUN: %clang -### -c -target x86_64 -gsplit-dwarf %s 2>&1 | FileCheck %s 
--check-prefix=G0
 
+/// -gno-split-dwarf disables debug fission.
+// RUN: %clang -### -c -target x86_64 -gsplit-dwarf -g -gno-split-dwarf %s 
2>&1 | FileCheck %s --check-prefix=NOSPLIT
+// RUN: %clang -### -c -target x86_64 -gsplit-dwarf=single -g -gno-split-dwarf 
%s 2>&1 | FileCheck %s --check-prefix=NOSPLIT
+// RUN: %clang -### -c -target x86_64 -gno-split-dwarf -g -gsplit-dwarf %s 
2>&1 | FileCheck %s --check-prefixes=NOINLINE,SPLIT
+
+// NOSPLIT: "-debug-info-kind=limited"
+// NOSPLIT-NOT: "-split-dwarf
+
 /// Test -gsplit-dwarf=single.
 // RUN: %clang -### -c -target x86_64 -gsplit-dwarf=single -g %s 2>&1 | 
FileCheck %s --check-prefix=SINGLE
 
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -3705,9 +3705,9 @@
 
 static DwarfFissionKind getDebugFissionKind(const Driver &D,
 const ArgList &Args, Arg *&Arg) {
-  Arg =
-  Args.getLastArg(options::OPT_gsplit_dwarf, options::OPT_gsplit_dwarf_EQ);
-  if (!Arg)
+  Arg = Args.getLastArg(options::OPT_gsplit_dwarf, 
options::OPT_gsplit_dwarf_EQ,
+options::OPT_gno_split_dwarf);
+  if (!Arg || Arg->getOption().matches(options::OPT_gno_split_dwarf))
 return DwarfFissionKind::None;
 
   if (Arg->getOption().matches(options::OPT_gsplit_dwarf))
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -2208,6 +2208,7 @@
 def gsplit_dwarf_EQ : Joined<["-"], "gsplit-dwarf=">, Group,
   HelpText<"Set DWARF fission mode to either 'split' or 'single'">,
   Values<"split,single">;
+def gno_split_dwarf : Flag<["-"], "gno-split-dwarf">, Group;
 def ggnu_pubnames : Flag<["-"], "ggnu-pubnames">, Group, 
Flags<[CC1Option]>;
 def gno_gnu_pubnames : Flag<["-"], "gno-gnu-pubnames">, Group;
 def gpubnames : Flag<["-"], "gpubnames">, Group, 
Flags<[CC1Option]>;


Index: clang/test/Driver/split-debug.c
===
--- clang/test/Driver/split-debug.c
+++ clang/test/Driver/split-debug.c
@@ -17,12 +17,21 @@
 // RUN: %clang -### -c -target amdgcn-amd-amdhsa -gsplit-dwarf -g %s 2>&1 | FileCheck %s --check-prefix=SPLIT
 
 /// -gsplit-dwarf is a no-op on a non-ELF platform.
-// RUN: %clang -### -c -target x86_64-apple-darwin  -gsplit-dwarf -g %s 2>&1 | FileCheck %s --check-prefix=NOSPLIT
-// NOSPLIT-NOT: "-split-dwarf
+// RUN: %clang -### -c -target x86_64-apple-darwin  -gsplit-dwarf -g %s 2>&1 | FileCheck %s --check-prefix=DARWIN
+// DARWIN: "-debug-info-kind=standalone"
+// DARWIN-NOT: "-split-dwarf
 
 /// -gsplit-dwarf is a no-op if no -g is specified.
 // RUN: %clang -### -c -target x86_64 -gsplit-dwarf %s 2>&1 | FileCheck %s --check-prefix=G0
 
+/// -gno-split-dwarf disables debug fission.
+// RUN: %clang -### -c -target x86_64 -gsplit-dwarf -g -gno-split-dwarf %s 2>&1 | FileCheck %s --check-prefix=NOSPLIT
+// RUN: %clang -### -c -target x86_64 -gsplit-dwarf=single -g -gno-split-dwarf %s 2>&1 | FileCheck %s --check-prefix=NOSPLIT
+// RUN: %clang -### -c -target x86_64 -gno-split-dwarf -g -gsplit-dwarf %s 2>&1 | FileCheck %s --check-prefixes=NOINLINE,SPLIT
+
+// NOSPLIT: "-debug-info-kind=limited"
+// NOSPLIT-NOT: "-split-dwar

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

[Kuan Hsu Chen (Zakk) via Phabricator via cfe-commits]

khchen added inline comments.



Comment at: clang/include/clang/Basic/RISCVVTypes.def:67
+RVV_VECTOR_TYPE_INT("__rvv_int8m2_t",  RvvInt8m2,  RvvInt8m2Ty,  16,  8, 1, 
true)
+RVV_VECTOR_TYPE_INT("__rvv_int8m4_t",  RvvInt8m4,  RvvInt8m4Ty,  32,  8, 1, 
true)
+RVV_VECTOR_TYPE_INT("__rvv_int8m8_t",  RvvInt8m8,  RvvInt8m8Ty,  64,  8, 1, 
true)

evandro wrote:
> craig.topper wrote:
> > craig.topper wrote:
> > > jrtc27 wrote:
> > > > liaolucy wrote:
> > > > > RISC-V V has too many types, more than 200. All types use builtin 
> > > > > types? Is it possible to reduce the number of builtin types?
> > > > Indeed this is madness, what's wrong with just using 
> > > > `__attribute__((vector_size(n)))` on the right type? We should not be 
> > > > encouraging people to write code with architecture-specific types... 
> > > > but if we _really_ need these because RISC-V GCC decided this is how 
> > > > RISC-V V is going to look them can we not just shove them all in a 
> > > > header as typedef's for the architecture-independent attributed types 
> > > > and push that complexity out of the compiler itself?
> > > We are using  to specify types in IR. The size of the 
> > > fixed part is being used to control the LMUL parameter. There is 
> > > currently no way to spell a scalable vector type in C in a generic way.
> > > 
> > > Alternatively I guess we could make LMUL a parameter to the intrinsic and 
> > > create the scalable IR types in the frontend based on it?
> > I do wonder why we bothered to have signed and unsigned types. The 
> > signedness of the operation should be carried in the intrinsic name.
> Some integer operations distinguish between signed and unsigned.  
> I do wonder why we bothered to have signed and unsigned types. The signedness 
> of the operation should be carried in the intrinsic name.

I think the only good thing for supporting both signed and unsigned type is 
that are more readable and compiler can does type checking for programmer. 

maybe the alternative way is changing intrinsic naming rule like 

```
vint32m1_t a, b, c;
a = vadd_i32m1(b, c);
vuint32m1_t a, b, c;
a = vadd_u32m1(b, c);
vfloat32m1_t a, b, c;
a = vadd_f32m1(b, c);
```


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

[Olivier Giroux via Phabricator via cfe-commits]

__simt__ added a comment.

I think Jonathan is asking whether there is a match in the gray areas.

The two cases people bring up most:

1. Unions, where the padding overlaps for all the possible active members.
2. Tail padding, up to the allocator granularity / alignment size.

If the implementation-specific builtins don't match on these, then maybe they 
should have different names, is his argument I think.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87974

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

[PATCH] D87974: [Builtin] Add __builtin_zero_non_value_bits.

[Zoe Carver via Phabricator via cfe-commits]

zoecarver added a comment.

> If the implementation-specific builtins don't match on these, then maybe they 
> should have different names, is his argument I think.

That's a fair point. And I agree, if they don't match, maybe it would be best 
to have different names. I'm hoping that we can all agree on how to handle 
these gray areas, though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87974

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

[PATCH] D92436: [Time-report] Add a flag -ftime-report={per-pass,per-pass-run} to control the pass timing aggregation

[Yuanfang Chen via Phabricator via cfe-commits]

ychen updated this revision to Diff 310079.
ychen marked an inline comment as done.
ychen added a comment.

- `clang/test/Misc/time-passes.c`: add -fno-experimental-new-pass-manager
- `llvm/test/Other/time-passes.ll`: makes sure each pass has one entry.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92436

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/Options.td
  clang/include/clang/Frontend/FrontendOptions.h
  clang/include/clang/Frontend/Utils.h
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/CodeGen/CodeGenAction.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CMakeLists.txt
  clang/lib/Frontend/CompilerInstance.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Frontend/FrontendTiming.cpp
  clang/test/Driver/time-report.c
  clang/test/Misc/time-passes.c
  llvm/include/llvm/IR/PassTimingInfo.h
  llvm/include/llvm/Pass.h
  llvm/lib/IR/PassTimingInfo.cpp
  llvm/test/Other/time-passes.ll

Index: llvm/test/Other/time-passes.ll
===
--- llvm/test/Other/time-passes.ll
+++ llvm/test/Other/time-passes.ll
@@ -1,9 +1,15 @@
 ; RUN: opt -enable-new-pm=0 < %s -disable-output -instcombine -instcombine -licm -time-passes 2>&1 | FileCheck %s --check-prefix=TIME --check-prefix=TIME-LEGACY
 ; RUN: opt -enable-new-pm=0 < %s -disable-output -instcombine -instcombine -licm -licm -time-passes 2>&1 | FileCheck %s --check-prefix=TIME --check-prefix=TIME-LEGACY --check-prefix=TIME-DOUBLE-LICM-LEGACY
-; RUN: opt < %s -disable-output -passes='instcombine,instcombine,loop(licm)' -time-passes 2>&1 | FileCheck %s --check-prefix=TIME --check-prefix=TIME-NEW
-; RUN: opt < %s -disable-output -passes='instcombine,loop(licm),instcombine,loop(licm)' -time-passes 2>&1 | FileCheck %s --check-prefix=TIME --check-prefix=TIME-NEW -check-prefix=TIME-DOUBLE-LICM-NEW
 ; RUN: opt < %s -disable-output -passes='default' -time-passes 2>&1 | FileCheck %s --check-prefix=TIME
 ;
+; For new pass manager, check that -time-passes-per-run emit one report for each pass run.
+; RUN: opt < %s -disable-output -passes='instcombine,instcombine,loop(licm)' -time-passes-per-run 2>&1 | FileCheck %s --check-prefix=TIME --check-prefix=TIME-NEW
+; RUN: opt < %s -disable-output -passes='instcombine,loop(licm),instcombine,loop(licm)' -time-passes-per-run 2>&1 | FileCheck %s --check-prefix=TIME --check-prefix=TIME-NEW -check-prefix=TIME-DOUBLE-LICM-NEW
+;
+; For new pass manager, check that -time-passes emit one report for each pass.
+; RUN: opt < %s -disable-output -passes='instcombine,instcombine,loop(licm)' -time-passes 2>&1 | FileCheck %s --check-prefixes=TIME,TIME-NEW-PER-PASS
+; RUN: opt < %s -disable-output -passes='instcombine,loop(licm),instcombine,loop(licm)' -time-passes 2>&1 | FileCheck %s --check-prefixes=TIME,TIME-NEW-PER-PASS
+;
 ; The following 4 test runs verify -info-output-file interaction (default goes to stderr, '-' goes to stdout).
 ; RUN: opt -enable-new-pm=0 < %s -disable-output -O2 -time-passes -info-output-file='-' 2>/dev/null | FileCheck %s --check-prefix=TIME
 ; RUN: opt < %s -disable-output -passes='default' -time-passes -info-output-file='-' 2>/dev/null | FileCheck %s --check-prefix=TIME
@@ -46,6 +52,24 @@
 ; TIME-NEW-DAG:  VerifierPass
 ; TIME-NEW-DAG:  DominatorTreeAnalysis
 ; TIME-NEW-DAG:  TargetLibraryAnalysis
+; TIME-NEW-PER-PASS-DAG:   InstCombinePass
+; TIME-NEW-PER-PASS-DAG:   LICMPass
+; TIME-NEW-PER-PASS-DAG:   LCSSAPass
+; TIME-NEW-PER-PASS-DAG:   LoopSimplifyPass
+; TIME-NEW-PER-PASS-DAG:   ScalarEvolutionAnalysis
+; TIME-NEW-PER-PASS-DAG:   LoopAnalysis
+; TIME-NEW-PER-PASS-DAG:   VerifierPass
+; TIME-NEW-PER-PASS-DAG:   DominatorTreeAnalysis
+; TIME-NEW-PER-PASS-DAG:   TargetLibraryAnalysis
+; TIME-NEW-PER-PASS-NOT:   InstCombinePass #
+; TIME-NEW-PER-PASS-NOT:   LICMPass #
+; TIME-NEW-PER-PASS-NOT:   LCSSAPass #
+; TIME-NEW-PER-PASS-NOT:   LoopSimplifyPass #
+; TIME-NEW-PER-PASS-NOT:   ScalarEvolutionAnalysis #
+; TIME-NEW-PER-PASS-NOT:   LoopAnalysis #
+; TIME-NEW-PER-PASS-NOT:   VerifierPass #
+; TIME-NEW-PER-PASS-NOT:   DominatorTreeAnalysis #
+; TIME-NEW-PER-PASS-NOT:   TargetLibraryAnalysis #
 ; TIME: Total{{$}}
 
 define i32 @foo() {
Index: llvm/lib/IR/PassTimingInfo.cpp
===
--- llvm/lib/IR/PassTimingInfo.cpp
+++ llvm/lib/IR/PassTimingInfo.cpp
@@ -35,11 +35,17 @@
 namespace llvm {
 
 bool TimePassesIsEnabled = false;
+bool TimePassesPerRun = false;
 
 static cl::opt EnableTiming(
 "time-passes", cl::location(TimePassesIsEnabled), cl::Hidden,
 cl::desc("Time each pass, printing elapsed time for each on exit"));
 
+static cl::opt EnableTimingPerRun(
+"time-passes-per-run", cl::location(TimePassesPerRun), cl::Hidden,
+cl::desc("Time each pass run, printing elapsed time for each run on exit"),
+cl::callback([](const bool 

[PATCH] D92436: [Time-report] Add a flag -ftime-report={per-pass,per-pass-run} to control the pass timing aggregation

[Yuanfang Chen via Phabricator via cfe-commits]

ychen added inline comments.



Comment at: llvm/lib/IR/PassTimingInfo.cpp:47
+cl::desc("Time each pass run, printing elapsed time for each run on exit"),
+cl::callback([](const bool &) { TimePassesIsEnabled = true; }));
+

aeubanks wrote:
> is this necessary?
Yeah, it is. `TimePassesPerRun` (corresponding to TimePassesHandler::PerRun) is 
not used if `TimePassesIsEnabled` is false.



Comment at: llvm/test/Other/time-passes.ll:55
 ; TIME-NEW-DAG:  TargetLibraryAnalysis
+; TIME-NEW-PER-PASS-DAG:   InstCombinePass
+; TIME-NEW-PER-PASS-DAG:   LICMPass

aeubanks wrote:
> check that this doesn't appear multiple times under TIME-NEW-PER-PASS?
This is important. Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92436

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

[clang] 6e614b0 - [NFC][MSan] Round up OffsetPtr in PoisonMembers

[Vitaly Buka via cfe-commits]

Author: Vitaly Buka
Date: 2020-12-07T19:57:49-08:00
New Revision: 6e614b0c7ed3a9a66428f342bf2a4b3700525395

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

LOG: [NFC][MSan] Round up OffsetPtr in PoisonMembers

getFieldOffset(layoutStartOffset)  is expected to point to the first trivial
field or the one which follows non-trivial. So it must be byte aligned already.
However this is not obvious without assumptions about callers.
This patch will avoid the need in such assumptions.

Depends on D92727.

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

Added: 


Modified: 
clang/lib/CodeGen/CGClass.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGClass.cpp b/clang/lib/CodeGen/CGClass.cpp
index b900ed70152e..1c32929414a5 100644
--- a/clang/lib/CodeGen/CGClass.cpp
+++ b/clang/lib/CodeGen/CGClass.cpp
@@ -18,6 +18,7 @@
 #include "TargetInfo.h"
 #include "clang/AST/Attr.h"
 #include "clang/AST/CXXInheritance.h"
+#include "clang/AST/CharUnits.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/EvaluatedExprVisitor.h"
 #include "clang/AST/RecordLayout.h"
@@ -1729,37 +1730,35 @@ namespace {
 /// \param layoutEndOffset index of the ASTRecordLayout field to
 /// end poisoning (exclusive)
 void PoisonMembers(CodeGenFunction &CGF, unsigned layoutStartOffset,
- unsigned layoutEndOffset) {
+   unsigned layoutEndOffset) {
   ASTContext &Context = CGF.getContext();
   const ASTRecordLayout &Layout =
   Context.getASTRecordLayout(Dtor->getParent());
 
-  llvm::ConstantInt *OffsetSizePtr = llvm::ConstantInt::get(
-  CGF.SizeTy,
-  Context.toCharUnitsFromBits(Layout.getFieldOffset(layoutStartOffset))
-  .getQuantity());
+  // It's a first trivia field so it should be at the begining of char,
+  // still round up start offset just in case.
+  CharUnits PoisonStart =
+  Context.toCharUnitsFromBits(Layout.getFieldOffset(layoutStartOffset) 
+
+  Context.getCharWidth() - 1);
+  llvm::ConstantInt *OffsetSizePtr =
+  llvm::ConstantInt::get(CGF.SizeTy, PoisonStart.getQuantity());
 
   llvm::Value *OffsetPtr = CGF.Builder.CreateGEP(
   CGF.Builder.CreateBitCast(CGF.LoadCXXThis(), CGF.Int8PtrTy),
   OffsetSizePtr);
 
-  CharUnits::QuantityType PoisonSize;
+  CharUnits PoisonEnd;
   if (layoutEndOffset >= Layout.getFieldCount()) {
-PoisonSize = Layout.getNonVirtualSize().getQuantity() -
- Context.toCharUnitsFromBits(
-Layout.getFieldOffset(layoutStartOffset))
- .getQuantity();
+PoisonEnd = Layout.getNonVirtualSize();
   } else {
-PoisonSize = Context.toCharUnitsFromBits(
-Layout.getFieldOffset(layoutEndOffset) -
-Layout.getFieldOffset(layoutStartOffset))
- .getQuantity();
+PoisonEnd =
+
Context.toCharUnitsFromBits(Layout.getFieldOffset(layoutEndOffset));
   }
-
-  if (PoisonSize == 0)
+  CharUnits PoisonSize = PoisonEnd - PoisonStart;
+  if (!PoisonSize.isPositive())
 return;
 
-  EmitSanitizerDtorCallback(CGF, OffsetPtr, PoisonSize);
+  EmitSanitizerDtorCallback(CGF, OffsetPtr, PoisonSize.getQuantity());
 }
   };
 



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

[PATCH] D92728: [NFC][MSan] Round up OffsetPtr in PoisonMembers

[Vitaly Buka 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 rG6e614b0c7ed3: [NFC][MSan] Round up OffsetPtr in 
PoisonMembers (authored by vitalybuka).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92728

Files:
  clang/lib/CodeGen/CGClass.cpp


Index: clang/lib/CodeGen/CGClass.cpp
===
--- clang/lib/CodeGen/CGClass.cpp
+++ clang/lib/CodeGen/CGClass.cpp
@@ -18,6 +18,7 @@
 #include "TargetInfo.h"
 #include "clang/AST/Attr.h"
 #include "clang/AST/CXXInheritance.h"
+#include "clang/AST/CharUnits.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/EvaluatedExprVisitor.h"
 #include "clang/AST/RecordLayout.h"
@@ -1729,37 +1730,35 @@
 /// \param layoutEndOffset index of the ASTRecordLayout field to
 /// end poisoning (exclusive)
 void PoisonMembers(CodeGenFunction &CGF, unsigned layoutStartOffset,
- unsigned layoutEndOffset) {
+   unsigned layoutEndOffset) {
   ASTContext &Context = CGF.getContext();
   const ASTRecordLayout &Layout =
   Context.getASTRecordLayout(Dtor->getParent());
 
-  llvm::ConstantInt *OffsetSizePtr = llvm::ConstantInt::get(
-  CGF.SizeTy,
-  Context.toCharUnitsFromBits(Layout.getFieldOffset(layoutStartOffset))
-  .getQuantity());
+  // It's a first trivia field so it should be at the begining of char,
+  // still round up start offset just in case.
+  CharUnits PoisonStart =
+  Context.toCharUnitsFromBits(Layout.getFieldOffset(layoutStartOffset) 
+
+  Context.getCharWidth() - 1);
+  llvm::ConstantInt *OffsetSizePtr =
+  llvm::ConstantInt::get(CGF.SizeTy, PoisonStart.getQuantity());
 
   llvm::Value *OffsetPtr = CGF.Builder.CreateGEP(
   CGF.Builder.CreateBitCast(CGF.LoadCXXThis(), CGF.Int8PtrTy),
   OffsetSizePtr);
 
-  CharUnits::QuantityType PoisonSize;
+  CharUnits PoisonEnd;
   if (layoutEndOffset >= Layout.getFieldCount()) {
-PoisonSize = Layout.getNonVirtualSize().getQuantity() -
- Context.toCharUnitsFromBits(
-Layout.getFieldOffset(layoutStartOffset))
- .getQuantity();
+PoisonEnd = Layout.getNonVirtualSize();
   } else {
-PoisonSize = Context.toCharUnitsFromBits(
-Layout.getFieldOffset(layoutEndOffset) -
-Layout.getFieldOffset(layoutStartOffset))
- .getQuantity();
+PoisonEnd =
+
Context.toCharUnitsFromBits(Layout.getFieldOffset(layoutEndOffset));
   }
-
-  if (PoisonSize == 0)
+  CharUnits PoisonSize = PoisonEnd - PoisonStart;
+  if (!PoisonSize.isPositive())
 return;
 
-  EmitSanitizerDtorCallback(CGF, OffsetPtr, PoisonSize);
+  EmitSanitizerDtorCallback(CGF, OffsetPtr, PoisonSize.getQuantity());
 }
   };
 


Index: clang/lib/CodeGen/CGClass.cpp
===
--- clang/lib/CodeGen/CGClass.cpp
+++ clang/lib/CodeGen/CGClass.cpp
@@ -18,6 +18,7 @@
 #include "TargetInfo.h"
 #include "clang/AST/Attr.h"
 #include "clang/AST/CXXInheritance.h"
+#include "clang/AST/CharUnits.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/EvaluatedExprVisitor.h"
 #include "clang/AST/RecordLayout.h"
@@ -1729,37 +1730,35 @@
 /// \param layoutEndOffset index of the ASTRecordLayout field to
 /// end poisoning (exclusive)
 void PoisonMembers(CodeGenFunction &CGF, unsigned layoutStartOffset,
- unsigned layoutEndOffset) {
+   unsigned layoutEndOffset) {
   ASTContext &Context = CGF.getContext();
   const ASTRecordLayout &Layout =
   Context.getASTRecordLayout(Dtor->getParent());
 
-  llvm::ConstantInt *OffsetSizePtr = llvm::ConstantInt::get(
-  CGF.SizeTy,
-  Context.toCharUnitsFromBits(Layout.getFieldOffset(layoutStartOffset))
-  .getQuantity());
+  // It's a first trivia field so it should be at the begining of char,
+  // still round up start offset just in case.
+  CharUnits PoisonStart =
+  Context.toCharUnitsFromBits(Layout.getFieldOffset(layoutStartOffset) +
+  Context.getCharWidth() - 1);
+  llvm::ConstantInt *OffsetSizePtr =
+  llvm::ConstantInt::get(CGF.SizeTy, PoisonStart.getQuantity());
 
   llvm::Value *OffsetPtr = CGF.Builder.CreateGEP(
   CGF.Builder.CreateBitCast(CGF.LoadCXXThis(), CGF.Int8PtrTy),
   OffsetSizePtr);
 
-  CharUnits::QuantityType PoisonSize;
+  CharUnits PoisonEnd;
   if (layoutEndOffset >= Layout.getFieldCount()) {
-PoisonSize =

[PATCH] D92436: [Time-report] Add a flag -ftime-report={per-pass,per-pass-run} to control the pass timing aggregation

[Arthur Eubanks via Phabricator via cfe-commits]

aeubanks added inline comments.



Comment at: llvm/lib/IR/PassTimingInfo.cpp:47
+cl::desc("Time each pass run, printing elapsed time for each run on exit"),
+cl::callback([](const bool &) { TimePassesIsEnabled = true; }));
+

ychen wrote:
> aeubanks wrote:
> > is this necessary?
> Yeah, it is. `TimePassesPerRun` (corresponding to TimePassesHandler::PerRun) 
> is not used if `TimePassesIsEnabled` is false.
Whoops, didn't see that this was touching two separate variables


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92436

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

[PATCH] D92810: [clang-tidy] Recognize single character needles for absl::StrContains.

[Chris Kennelly via Phabricator via cfe-commits]

ckennelly created this revision.
ckennelly added reviewers: EricWF, ymandel, hokein.
Herald added a subscriber: xazax.hun.
ckennelly requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Commit fbdff6f3ae0b in the Abseil tree adds an overload for
absl::StrContains to accept a single character needle for optimized
lookups.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D92810

Files:
  clang-tools-extra/clang-tidy/abseil/StringFindStrContainsCheck.cpp
  clang-tools-extra/test/clang-tidy/checkers/abseil-string-find-str-contains.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/abseil-string-find-str-contains.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/abseil-string-find-str-contains.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/abseil-string-find-str-contains.cpp
@@ -226,17 +226,21 @@
   // CHECK-FIXES: {{^[[:space:]]*}}!absl::StrContains(asv, cc);{{$}}
 }
 
-// Confirms that it does *not* match when the parameter to find() is a char,
-// because absl::StrContains is not implemented for char.
-void no_char_param_tests() {
+void char_param_tests() {
   std::string ss;
   ss.find('c') == std::string::npos;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use !absl::StrContains instead of
+  // CHECK-FIXES: {{^[[:space:]]*}}!absl::StrContains(ss, 'c');{{$}}
 
   std::string_view ssv;
   ssv.find('c') == std::string_view::npos;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use !absl::StrContains instead of
+  // CHECK-FIXES: {{^[[:space:]]*}}!absl::StrContains(ssv, 'c');{{$}}
 
   absl::string_view asv;
   asv.find('c') == absl::string_view::npos;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use !absl::StrContains instead of
+  // CHECK-FIXES: {{^[[:space:]]*}}!absl::StrContains(asv, 'c');{{$}}
 }
 
 #define FOO(a, b, c, d) ((a).find(b) == std::string::npos ? (c) : (d))
Index: clang-tools-extra/clang-tidy/abseil/StringFindStrContainsCheck.cpp
===
--- clang-tools-extra/clang-tidy/abseil/StringFindStrContainsCheck.cpp
+++ clang-tools-extra/clang-tidy/abseil/StringFindStrContainsCheck.cpp
@@ -31,6 +31,8 @@
 using ::clang::transformer::node;
 using ::clang::transformer::RewriteRule;
 
+AST_MATCHER(Type, isCharType) { return Node.isCharType(); }
+
 static const char DefaultStringLikeClasses[] = "::std::basic_string;"
"::std::basic_string_view;"
"::absl::string_view";
@@ -58,13 +60,15 @@
   hasUnqualifiedDesugaredType(recordType(hasDeclaration(StringLikeClass)));
   auto CharStarType =
   hasUnqualifiedDesugaredType(pointerType(pointee(isAnyCharacter(;
+  auto CharType = hasUnqualifiedDesugaredType(isCharType());
   auto StringNpos = declRefExpr(
   to(varDecl(hasName("npos"), hasDeclContext(StringLikeClass;
   auto StringFind = cxxMemberCallExpr(
   callee(cxxMethodDecl(
   hasName("find"),
-  hasParameter(0, parmVarDecl(anyOf(hasType(StringType),
-hasType(CharStarType)),
+  hasParameter(
+  0, parmVarDecl(anyOf(hasType(StringType), hasType(CharStarType),
+   hasType(CharType)),
   on(hasType(StringType)), hasArgument(0, 
expr().bind("parameter_to_find")),
   anyOf(hasArgument(1, integerLiteral(equals(0))),
 hasArgument(1, cxxDefaultArgExpr())),


Index: clang-tools-extra/test/clang-tidy/checkers/abseil-string-find-str-contains.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/abseil-string-find-str-contains.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/abseil-string-find-str-contains.cpp
@@ -226,17 +226,21 @@
   // CHECK-FIXES: {{^[[:space:]]*}}!absl::StrContains(asv, cc);{{$}}
 }
 
-// Confirms that it does *not* match when the parameter to find() is a char,
-// because absl::StrContains is not implemented for char.
-void no_char_param_tests() {
+void char_param_tests() {
   std::string ss;
   ss.find('c') == std::string::npos;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use !absl::StrContains instead of
+  // CHECK-FIXES: {{^[[:space:]]*}}!absl::StrContains(ss, 'c');{{$}}
 
   std::string_view ssv;
   ssv.find('c') == std::string_view::npos;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use !absl::StrContains instead of
+  // CHECK-FIXES: {{^[[:space:]]*}}!absl::StrContains(ssv, 'c');{{$}}
 
   absl::string_view asv;
   asv.find('c') == absl::string_view::npos;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use !absl::StrContains instead of
+  // CHECK-FIXES: {{^[[:space:]]*}}!absl::StrContains(asv, 'c');{{$}}
 }
 
 #define FOO(a, b, c, d) ((a).find(b) == std::string::npos ? (c) : (d))
Index: clang-tools-extra/clang-tidy/abseil/StringFindStrContainsCheck.cpp
==

[PATCH] D92812: [X86] AMD Znver3 (Family 19H) Enablement

[Ganesh Gopalasubramanian via Phabricator via cfe-commits]

GGanesh created this revision.
GGanesh added reviewers: RKSimon, craig.topper, lebedev.ri, courbet.
GGanesh added projects: LLVM, clang.
Herald added subscribers: mstojanovic, pengfei, jfb, gbedwell, hiraditya.
Herald added a reviewer: andreadb.
GGanesh requested review of this revision.
Herald added subscribers: llvm-commits, Sanitizers, cfe-commits.
Herald added a project: Sanitizers.

This patch enables the new AMD family 19H architecture

1. Introduced a new command line switch -march=”znver3”
2. Following ISAs are added to znver3 arch. New ISAs are added to Instruction 
tables.

•   INVPCID,
•   PKU
•   VAES,
•   VPCLMULQDQ
•   SNP
•   INVLPGB
•   TLBSYNC

3. Enables the "march=native" detection to znver3.
4. Adds testcases to test the latency, throughput and execution pipelines.
5. Enables llvm exegesis tool.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D92812

Files:
  clang/lib/Basic/Targets/X86.cpp
  clang/test/Driver/x86-march.c
  clang/test/Frontend/x86-target-cpu.c
  clang/test/Preprocessor/predefined-arch-macros.c
  compiler-rt/lib/builtins/cpu_model.c
  llvm/lib/Target/X86/X86.td
  llvm/lib/Target/X86/X86InstrInfo.td
  llvm/lib/Target/X86/X86InstrSNP.td
  llvm/lib/Target/X86/X86PfmCounters.td
  llvm/lib/Target/X86/X86ScheduleZnver3.td
  llvm/lib/Target/X86/X86Subtarget.h
  llvm/test/CodeGen/X86/cpus-amd.ll
  llvm/test/CodeGen/X86/lzcnt-zext-cmp.ll
  llvm/test/CodeGen/X86/slow-unaligned-mem.ll
  llvm/test/CodeGen/X86/x86-64-double-shifts-var.ll
  llvm/test/MC/Disassembler/X86/x86-32.txt
  llvm/test/MC/X86/SNP-32.s
  llvm/test/MC/X86/SNP-64.s
  llvm/test/tools/llvm-mca/X86/Znver3/partial-reg-update-2.s
  llvm/test/tools/llvm-mca/X86/Znver3/partial-reg-update-3.s
  llvm/test/tools/llvm-mca/X86/Znver3/partial-reg-update-4.s
  llvm/test/tools/llvm-mca/X86/Znver3/partial-reg-update-5.s
  llvm/test/tools/llvm-mca/X86/Znver3/partial-reg-update-6.s
  llvm/test/tools/llvm-mca/X86/Znver3/partial-reg-update-7.s
  llvm/test/tools/llvm-mca/X86/Znver3/partial-reg-update.s
  llvm/test/tools/llvm-mca/X86/Znver3/resources-adx.s
  llvm/test/tools/llvm-mca/X86/Znver3/resources-aes.s
  llvm/test/tools/llvm-mca/X86/Znver3/resources-avx1.s
  llvm/test/tools/llvm-mca/X86/Znver3/resources-avx2.s
  llvm/test/tools/llvm-mca/X86/Znver3/resources-bmi1.s
  llvm/test/tools/llvm-mca/X86/Znver3/resources-bmi2.s
  llvm/test/tools/llvm-mca/X86/Znver3/resources-clflushopt.s
  llvm/test/tools/llvm-mca/X86/Znver3/resources-clzero.s
  llvm/test/tools/llvm-mca/X86/Znver3/resources-cmov.s
  llvm/test/tools/llvm-mca/X86/Znver3/resources-cmpxchg.s
  llvm/test/tools/llvm-mca/X86/Znver3/resources-f16c.s
  llvm/test/tools/llvm-mca/X86/Znver3/resources-fma.s
  llvm/test/tools/llvm-mca/X86/Znver3/resources-fsgsbase.s
  llvm/test/tools/llvm-mca/X86/Znver3/resources-lea.s
  llvm/test/tools/llvm-mca/X86/Znver3/resources-lzcnt.s
  llvm/test/tools/llvm-mca/X86/Znver3/resources-mmx.s
  llvm/test/tools/llvm-mca/X86/Znver3/resources-movbe.s
  llvm/test/tools/llvm-mca/X86/Znver3/resources-mwaitx.s
  llvm/test/tools/llvm-mca/X86/Znver3/resources-pclmul.s
  llvm/test/tools/llvm-mca/X86/Znver3/resources-popcnt.s
  llvm/test/tools/llvm-mca/X86/Znver3/resources-prefetchw.s
  llvm/test/tools/llvm-mca/X86/Znver3/resources-rdrand.s
  llvm/test/tools/llvm-mca/X86/Znver3/resources-rdseed.s
  llvm/test/tools/llvm-mca/X86/Znver3/resources-sha.s
  llvm/test/tools/llvm-mca/X86/Znver3/resources-sse1.s
  llvm/test/tools/llvm-mca/X86/Znver3/resources-sse2.s
  llvm/test/tools/llvm-mca/X86/Znver3/resources-sse3.s
  llvm/test/tools/llvm-mca/X86/Znver3/resources-sse41.s
  llvm/test/tools/llvm-mca/X86/Znver3/resources-sse42.s
  llvm/test/tools/llvm-mca/X86/Znver3/resources-sse4a.s
  llvm/test/tools/llvm-mca/X86/Znver3/resources-ssse3.s
  llvm/test/tools/llvm-mca/X86/Znver3/resources-x86_32.s
  llvm/test/tools/llvm-mca/X86/Znver3/resources-x86_64.s
  llvm/test/tools/llvm-mca/X86/Znver3/resources-x87.s

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

[PATCH] D92812: [X86] AMD Znver3 (Family 19H) Enablement

[Craig Topper via Phabricator via cfe-commits]

craig.topper added a comment.

Please upload the patch with full context


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92812

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

[PATCH] D92812: [X86] AMD Znver3 (Family 19H) Enablement

[Craig Topper via Phabricator via cfe-commits]

craig.topper added a subscriber: bkramer.
craig.topper added inline comments.



Comment at: clang/lib/Basic/Targets/X86.cpp:548
 break;
-  case CK_ZNVER3:
-defineCPUMacros(Builder, "znver3");

Why is this being deleted?



Comment at: clang/lib/Basic/Targets/X86.cpp:1339
 case CK_ZNVER2:
-case CK_ZNVER3:
 // Deprecated

Same here?



Comment at: compiler-rt/lib/builtins/cpu_model.c:480
 break;
+  case 25:
+*Type = AMDFAM19H;

I thought this code was already added to compiler-rt by @bkramer 



Comment at: llvm/test/CodeGen/X86/cpus-amd.ll:30
 ; RUN: llc < %s -o /dev/null -mtriple=x86_64-unknown-unknown -mcpu=znver2 2>&1 
| FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
-; RUN: llc < %s -o /dev/null -mtriple=x86_64-unknown-unknown -mcpu=znver3 2>&1 
| FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
 

Why deleted?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92812

___
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

[Kito Cheng via Phabricator via cfe-commits]

kito-cheng added inline comments.



Comment at: clang/include/clang/Basic/RISCVVTypes.def:67
+RVV_VECTOR_TYPE_INT("__rvv_int8m2_t",  RvvInt8m2,  RvvInt8m2Ty,  16,  8, 1, 
true)
+RVV_VECTOR_TYPE_INT("__rvv_int8m4_t",  RvvInt8m4,  RvvInt8m4Ty,  32,  8, 1, 
true)
+RVV_VECTOR_TYPE_INT("__rvv_int8m8_t",  RvvInt8m8,  RvvInt8m8Ty,  64,  8, 1, 
true)

khchen wrote:
> evandro wrote:
> > craig.topper wrote:
> > > craig.topper wrote:
> > > > jrtc27 wrote:
> > > > > liaolucy wrote:
> > > > > > RISC-V V has too many types, more than 200. All types use builtin 
> > > > > > types? Is it possible to reduce the number of builtin types?
> > > > > Indeed this is madness, what's wrong with just using 
> > > > > `__attribute__((vector_size(n)))` on the right type? We should not be 
> > > > > encouraging people to write code with architecture-specific types... 
> > > > > but if we _really_ need these because RISC-V GCC decided this is how 
> > > > > RISC-V V is going to look them can we not just shove them all in a 
> > > > > header as typedef's for the architecture-independent attributed types 
> > > > > and push that complexity out of the compiler itself?
> > > > We are using  to specify types in IR. The size of the 
> > > > fixed part is being used to control the LMUL parameter. There is 
> > > > currently no way to spell a scalable vector type in C in a generic way.
> > > > 
> > > > Alternatively I guess we could make LMUL a parameter to the intrinsic 
> > > > and create the scalable IR types in the frontend based on it?
> > > I do wonder why we bothered to have signed and unsigned types. The 
> > > signedness of the operation should be carried in the intrinsic name.
> > Some integer operations distinguish between signed and unsigned.  
> > I do wonder why we bothered to have signed and unsigned types. The 
> > signedness of the operation should be carried in the intrinsic name.
> 
> I think the only good thing for supporting both signed and unsigned type is 
> that are more readable and compiler can does type checking for programmer. 
> 
> maybe the alternative way is changing intrinsic naming rule like 
> 
> ```
> vint32m1_t a, b, c;
> a = vadd_i32m1(b, c);
> vuint32m1_t a, b, c;
> a = vadd_u32m1(b, c);
> vfloat32m1_t a, b, c;
> a = vadd_f32m1(b, c);
> ```
One quick thought about this, if the concern is too much built-in types are 
introduced in clang, maybe we could add a new attribute like 
`__attribute__((vector_size(n)))`, maybe named 
`__attribute__((riscv_scaleble_vector("[1|2|4|8|f2|f4|f8]")))`? and use that to 
define vector types like `typedef int 
__attribute__((riscv_scaleble_vector("2"))) vintm2_t`.


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] D92078: [asan] Default to -asan-use-private-alias=1

[Vitaly Buka via Phabricator via cfe-commits]

vitalybuka added a subscriber: kcc.
vitalybuka added a comment.

I've chatted about that with @kcc and @eugenis . It seems the problem the patch 
is trying to solve is less important than regressions. Even with the current 
state when rare false ODR reports are possible it still useful.
It would be nice with -fsanitize-address-use-odr-indicator, but someone will 
need to figure-out what to do with few users broken by binary size increase.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92078

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

[clang] 5e85a2b - [PowerPC] Implement intrinsic for DARN instruction

[Qiu Chaofan via cfe-commits]

Author: Qiu Chaofan
Date: 2020-12-08T14:08:52+08:00
New Revision: 5e85a2ba1645c3edbf26bba096631fbd318ada47

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

LOG: [PowerPC] Implement intrinsic for DARN instruction

Instruction darn was introduced in ISA 3.0. It means 'Deliver A Random
Number'. The immediate number L means:

- L=0, the number is 32-bit (higher 32-bits are all-zero)
- L=1, the number is 'conditioned' (processed by hardware to reduce bias)
- L=2, the number is not conditioned, directly from noise source

GCC implements them in three separate intrinsics: __builtin_darn,
__builtin_darn_32 and __builtin_darn_raw. This patch implements the
same intrinsics. And this change also addresses Bugzilla PR39800.

Reviewed By: steven.zhang

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

Added: 
llvm/test/CodeGen/PowerPC/builtins-ppc-p9-darn.ll

Modified: 
clang/include/clang/Basic/BuiltinsPPC.def
clang/test/CodeGen/builtins-ppc.c
llvm/include/llvm/IR/IntrinsicsPowerPC.td
llvm/lib/Target/PowerPC/PPCInstr64Bit.td

Removed: 




diff  --git a/clang/include/clang/Basic/BuiltinsPPC.def 
b/clang/include/clang/Basic/BuiltinsPPC.def
index 78ce77043b6f..8975d126b897 100644
--- a/clang/include/clang/Basic/BuiltinsPPC.def
+++ b/clang/include/clang/Basic/BuiltinsPPC.def
@@ -638,6 +638,11 @@ BUILTIN(__builtin_cfuged, "ULLiULLiULLi", "")
 BUILTIN(__builtin_cntlzdm, "ULLiULLiULLi", "")
 BUILTIN(__builtin_cnttzdm, "ULLiULLiULLi", "")
 
+// Generate random number
+BUILTIN(__builtin_darn, "LLi", "")
+BUILTIN(__builtin_darn_raw, "LLi", "")
+BUILTIN(__builtin_darn_32, "i", "")
+
 // Vector int128 (un)pack
 BUILTIN(__builtin_unpack_vector_int128, "ULLiV1LLLii", "")
 BUILTIN(__builtin_pack_vector_int128, "V1LLLiULLiULLi", "")

diff  --git a/clang/test/CodeGen/builtins-ppc.c 
b/clang/test/CodeGen/builtins-ppc.c
index e30cdff3c8ce..0abd540013e2 100644
--- a/clang/test/CodeGen/builtins-ppc.c
+++ b/clang/test/CodeGen/builtins-ppc.c
@@ -36,3 +36,16 @@ void test_builtin_ppc_flm() {
   // CHECK: call double @llvm.ppc.setflm(double %1)
   res = __builtin_setflm(res);
 }
+
+void test_builtin_ppc_darn() {
+  volatile long res;
+  volatile int x;
+  // CHECK: call i64 @llvm.ppc.darn()
+  res = __builtin_darn();
+
+  // CHECK: call i64 @llvm.ppc.darnraw()
+  res = __builtin_darn_raw();
+
+  // CHECK: call i32 @llvm.ppc.darn32()
+  x = __builtin_darn_32();
+}

diff  --git a/llvm/include/llvm/IR/IntrinsicsPowerPC.td 
b/llvm/include/llvm/IR/IntrinsicsPowerPC.td
index 8db5c15fe761..d559c000fd93 100644
--- a/llvm/include/llvm/IR/IntrinsicsPowerPC.td
+++ b/llvm/include/llvm/IR/IntrinsicsPowerPC.td
@@ -70,6 +70,14 @@ let TargetPrefix = "ppc" in {  // All intrinsics start with 
"llvm.ppc.".
Intrinsic<[llvm_i64_ty], [llvm_i64_ty, llvm_i64_ty],
  [IntrNoMem]>;
 
+  // Generate a random number
+  def int_ppc_darn : GCCBuiltin<"__builtin_darn">,
+ Intrinsic<[llvm_i64_ty], [], [IntrNoMem]>;
+  def int_ppc_darnraw : GCCBuiltin<"__builtin_darn_raw">,
+ Intrinsic<[llvm_i64_ty], [], [IntrNoMem]>;
+  def int_ppc_darn32 : GCCBuiltin<"__builtin_darn_32">,
+ Intrinsic<[llvm_i32_ty], [], [IntrNoMem]>;
+
   // Bit permute doubleword
   def int_ppc_bpermd : GCCBuiltin<"__builtin_bpermd">,
Intrinsic<[llvm_i64_ty], [llvm_i64_ty, llvm_i64_ty],

diff  --git a/llvm/lib/Target/PowerPC/PPCInstr64Bit.td 
b/llvm/lib/Target/PowerPC/PPCInstr64Bit.td
index 82b868ec2b10..9265c513c031 100644
--- a/llvm/lib/Target/PowerPC/PPCInstr64Bit.td
+++ b/llvm/lib/Target/PowerPC/PPCInstr64Bit.td
@@ -1606,6 +1606,11 @@ def : Pat<(atomic_store_64 iaddrX4:$ptr, i64:$val), (STD 
 g8rc:$val, memrix:$ptr
 def : Pat<(atomic_store_64 xaddrX4:$ptr,  i64:$val), (STDX g8rc:$val, 
memrr:$ptr)>;
 
 let Predicates = [IsISA3_0] in {
+// DARN (deliver random number)
+// L=0 for 32-bit, L=1 for conditioned random, L=2 for raw random
+def : Pat<(int_ppc_darn32), (EXTRACT_SUBREG (DARN 0), sub_32)>;
+def : Pat<(int_ppc_darn), (DARN 1)>;
+def : Pat<(int_ppc_darnraw), (DARN 2)>;
 
 class X_L1_RA5_RB5 opcode, bits<10> xo, string opc, RegisterOperand ty,
InstrItinClass itin, list pattern>

diff  --git a/llvm/test/CodeGen/PowerPC/builtins-ppc-p9-darn.ll 
b/llvm/test/CodeGen/PowerPC/builtins-ppc-p9-darn.ll
new file mode 100644
index ..d53b442fef71
--- /dev/null
+++ b/llvm/test/CodeGen/PowerPC/builtins-ppc-p9-darn.ll
@@ -0,0 +1,37 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc < %s -verify-machineinstrs -mtriple powerpc64le -mcpu=pwr9 | 
FileCheck %s
+
+define i64 @raw() {
+; CHECK-LABEL: raw:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:darn 3

[PATCH] D92465: [PowerPC] Implement intrinsic for DARN instruction

[Qiu Chaofan via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG5e85a2ba1645: [PowerPC] Implement intrinsic for DARN 
instruction (authored by qiucf).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Changed prior to commit:
  https://reviews.llvm.org/D92465?vs=308882&id=310089#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92465

Files:
  clang/include/clang/Basic/BuiltinsPPC.def
  clang/test/CodeGen/builtins-ppc.c
  llvm/include/llvm/IR/IntrinsicsPowerPC.td
  llvm/lib/Target/PowerPC/PPCInstr64Bit.td
  llvm/test/CodeGen/PowerPC/builtins-ppc-p9-darn.ll

Index: llvm/test/CodeGen/PowerPC/builtins-ppc-p9-darn.ll
===
--- /dev/null
+++ llvm/test/CodeGen/PowerPC/builtins-ppc-p9-darn.ll
@@ -0,0 +1,37 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc < %s -verify-machineinstrs -mtriple powerpc64le -mcpu=pwr9 | FileCheck %s
+
+define i64 @raw() {
+; CHECK-LABEL: raw:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:darn 3, 2
+; CHECK-NEXT:blr
+entry:
+  %0 = call i64 @llvm.ppc.darnraw()
+  ret i64 %0
+}
+
+define i64 @conditioned() {
+; CHECK-LABEL: conditioned:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:darn 3, 1
+; CHECK-NEXT:blr
+entry:
+  %0 = call i64 @llvm.ppc.darn()
+  ret i64 %0
+}
+
+define signext i32 @word() {
+; CHECK-LABEL: word:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:darn 3, 0
+; CHECK-NEXT:extsw 3, 3
+; CHECK-NEXT:blr
+entry:
+  %0 = call i32 @llvm.ppc.darn32()
+  ret i32 %0
+}
+
+declare i64 @llvm.ppc.darn()
+declare i64 @llvm.ppc.darnraw()
+declare i32 @llvm.ppc.darn32()
Index: llvm/lib/Target/PowerPC/PPCInstr64Bit.td
===
--- llvm/lib/Target/PowerPC/PPCInstr64Bit.td
+++ llvm/lib/Target/PowerPC/PPCInstr64Bit.td
@@ -1606,6 +1606,11 @@
 def : Pat<(atomic_store_64 xaddrX4:$ptr,  i64:$val), (STDX g8rc:$val, memrr:$ptr)>;
 
 let Predicates = [IsISA3_0] in {
+// DARN (deliver random number)
+// L=0 for 32-bit, L=1 for conditioned random, L=2 for raw random
+def : Pat<(int_ppc_darn32), (EXTRACT_SUBREG (DARN 0), sub_32)>;
+def : Pat<(int_ppc_darn), (DARN 1)>;
+def : Pat<(int_ppc_darnraw), (DARN 2)>;
 
 class X_L1_RA5_RB5 opcode, bits<10> xo, string opc, RegisterOperand ty,
InstrItinClass itin, list pattern>
Index: llvm/include/llvm/IR/IntrinsicsPowerPC.td
===
--- llvm/include/llvm/IR/IntrinsicsPowerPC.td
+++ llvm/include/llvm/IR/IntrinsicsPowerPC.td
@@ -70,6 +70,14 @@
Intrinsic<[llvm_i64_ty], [llvm_i64_ty, llvm_i64_ty],
  [IntrNoMem]>;
 
+  // Generate a random number
+  def int_ppc_darn : GCCBuiltin<"__builtin_darn">,
+ Intrinsic<[llvm_i64_ty], [], [IntrNoMem]>;
+  def int_ppc_darnraw : GCCBuiltin<"__builtin_darn_raw">,
+ Intrinsic<[llvm_i64_ty], [], [IntrNoMem]>;
+  def int_ppc_darn32 : GCCBuiltin<"__builtin_darn_32">,
+ Intrinsic<[llvm_i32_ty], [], [IntrNoMem]>;
+
   // Bit permute doubleword
   def int_ppc_bpermd : GCCBuiltin<"__builtin_bpermd">,
Intrinsic<[llvm_i64_ty], [llvm_i64_ty, llvm_i64_ty],
Index: clang/test/CodeGen/builtins-ppc.c
===
--- clang/test/CodeGen/builtins-ppc.c
+++ clang/test/CodeGen/builtins-ppc.c
@@ -36,3 +36,16 @@
   // CHECK: call double @llvm.ppc.setflm(double %1)
   res = __builtin_setflm(res);
 }
+
+void test_builtin_ppc_darn() {
+  volatile long res;
+  volatile int x;
+  // CHECK: call i64 @llvm.ppc.darn()
+  res = __builtin_darn();
+
+  // CHECK: call i64 @llvm.ppc.darnraw()
+  res = __builtin_darn_raw();
+
+  // CHECK: call i32 @llvm.ppc.darn32()
+  x = __builtin_darn_32();
+}
Index: clang/include/clang/Basic/BuiltinsPPC.def
===
--- clang/include/clang/Basic/BuiltinsPPC.def
+++ clang/include/clang/Basic/BuiltinsPPC.def
@@ -638,6 +638,11 @@
 BUILTIN(__builtin_cntlzdm, "ULLiULLiULLi", "")
 BUILTIN(__builtin_cnttzdm, "ULLiULLiULLi", "")
 
+// Generate random number
+BUILTIN(__builtin_darn, "LLi", "")
+BUILTIN(__builtin_darn_raw, "LLi", "")
+BUILTIN(__builtin_darn_32, "i", "")
+
 // Vector int128 (un)pack
 BUILTIN(__builtin_unpack_vector_int128, "ULLiV1LLLii", "")
 BUILTIN(__builtin_pack_vector_int128, "V1LLLiULLiULLi", "")
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D92792: [clang] - Also look for devtoolset-10

[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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92792

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

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

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

In D92257#2435906 , @lebedev.ri wrote:

> In D92257#2435902 , @MyDeveloperDay 
> wrote:
>
>> In D92257#2435899 , 
>> @HazardyKnusperkeks wrote:
>>
>>> In D92257#2435701 , 
>>> @MyDeveloperDay wrote:
>>>
 Can I assume you need someone to land this for you?
>>>
>>> Yes I do. But I have a question, my last change is commited in your name, 
>>> that means git blame would blame it on you, right?
>>>
>>> You can set me as author:
>>> `Björn Schäpers `
>>> My Github Account is also called `HazardyKnusperkeks`.
>>
>> The process is that you add (https://llvm.org/docs/Contributing.html)
>>
>> Patch By: HazardyKnusperkeks
>>
>> to the commit message if the user doesn't have commit access, if you want 
>> your name against the blame then I recommend applying for commit access 
>> yourself.
>
> That is incorrect and does not represent the nowadays reality, i suggest that 
> you look up the docs.
>
>> let me know if you still want me to land this

Yes I agree I hadn’t seen that the process had changed,

This is one reason why I don’t like landing patches for others, this just 
confirms that in the future I will generally request people apply for commit 
access themselves.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92257

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

[PATCH] D92661: [RFC] Fix TLS and Coroutine

[Hongtao Yu via Phabricator via cfe-commits]

hoy added inline comments.



Comment at: llvm/include/llvm/IR/Intrinsics.td:1309
+// Intrinsic to obtain the address of a thread_local variable.
+def int_threadlocal : Intrinsic<[llvm_ptr_ty], [llvm_ptr_ty]>;
+

lxfind wrote:
> hoy wrote:
> > lxfind wrote:
> > > hoy wrote:
> > > > hoy wrote:
> > > > > With the intrinsic, can TLS variable reference in the same coroutine 
> > > > > or regular routine be DCE-ed anymore?
> > > > Sorry, I meant CSE-ed.
> > > Since the intrinsics does not have readnone attribute, it won't be CSE-ed 
> > > before CoroSplit.
> > > However after CoroSplit, it will be lowered back to the direct reference 
> > > of the TLS, and will be CSE-ed by latter passes.
> > > I can add a test function to demonstrate that too.
> > Sounds good. Can you please point out what optimization passes CSE-ed tls 
> > reference without this implementation? I'm wondering if those optimizations 
> > can be postponed to after CoroSplit. 
> To clarify, it wasn't just CSE that would merge the references of the same 
> TLS.
> For instance, without this patch, a reference to "tls_variable" will just be 
> "@tls_variable". For code like this:
> 
>   @tls_variable = internal thread_local global i32 0, align 4
> 
>   define i32* @foo(){
> ret i32* @tls_variable
>   }
>   
>   define void @bar() {
> %tls1 = call i32* @foo()
> ..coro.suspend..
> %tls2 = call i32* @foo()
> %cond = icmp eq i32* %tls1, %tls2
>   }
> 
> When foo() is inlined into bar(), all uses of %tls1 will be replaced with 
> @tls_variable.
Thanks for the explanation. I have a dumb question. Why isn't corosplit placed 
at the very beginning of the pipeline?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92661

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

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

[MyDeveloperDay via Phabricator via cfe-commits]

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

Could we consider dropping the maximum?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92257

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

[PATCH] D92157: [clangd] Add language metrics for recovery AST usage.

[Haojian Wu via Phabricator via cfe-commits]

hokein updated this revision to Diff 309817.
hokein marked 2 inline comments as done.
hokein added a comment.

address review comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92157

Files:
  clang-tools-extra/clangd/Selection.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
@@ -18,6 +18,7 @@
 namespace clang {
 namespace clangd {
 namespace {
+using ::testing::ElementsAreArray;
 using ::testing::UnorderedElementsAreArray;
 
 // Create a selection tree corresponding to a point or pair of points.
@@ -452,7 +453,8 @@
 if (Test.ranges().empty()) {
   // If no [[range]] is marked in the example, there should be no 
selection.
   EXPECT_FALSE(T.commonAncestor()) << C.Code << "\n" << T;
-  EXPECT_THAT(Tracer.takeMetric("selection_recovery"), testing::IsEmpty());
+  EXPECT_THAT(Tracer.takeMetric("selection_recovery", "C++"),
+  testing::IsEmpty());
 } else {
   // If there is an expected selection, common ancestor should exist
   // with the appropriate node type.
@@ -468,8 +470,8 @@
   // and no nodes outside it are selected.
   EXPECT_TRUE(verifyCommonAncestor(T.root(), T.commonAncestor(), C.Code))
   << C.Code;
-  EXPECT_THAT(Tracer.takeMetric("selection_recovery"),
-  testing::ElementsAreArray({0}));
+  EXPECT_THAT(Tracer.takeMetric("selection_recovery", "C++"),
+  ElementsAreArray({0}));
 }
   }
 }
@@ -494,10 +496,10 @@
   auto AST = TestTU::withCode(Annotations(Code).code()).build();
   trace::TestTracer Tracer;
   auto T = makeSelectionTree(Code, AST);
-  EXPECT_THAT(Tracer.takeMetric("selection_recovery"),
-  testing::ElementsAreArray({1}));
-  EXPECT_THAT(Tracer.takeMetric("selection_recovery_type"),
-  testing::ElementsAreArray({1}));
+  EXPECT_THAT(Tracer.takeMetric("selection_recovery", "C++"),
+  ElementsAreArray({1}));
+  EXPECT_THAT(Tracer.takeMetric("selection_recovery_type", "C++"),
+  ElementsAreArray({1}));
 }
 
 // FIXME: Doesn't select the binary operator node in
Index: clang-tools-extra/clangd/Selection.cpp
===
--- clang-tools-extra/clangd/Selection.cpp
+++ clang-tools-extra/clangd/Selection.cpp
@@ -38,21 +38,24 @@
 using ast_type_traits::DynTypedNode;
 
 // Measure the fraction of selections that were enabled by recovery AST.
-void recordMetrics(const SelectionTree &S) {
+void recordMetrics(const SelectionTree &S, const LangOptions &Lang) {
+  if (!trace::enabled())
+return;
+  const char *LanguageLabel = Lang.CPlusPlus ? "C++" : Lang.ObjC ? "ObjC" : 
"C";
   static constexpr trace::Metric SelectionUsedRecovery(
-  "selection_recovery", trace::Metric::Distribution);
-  static constexpr trace::Metric RecoveryType("selection_recovery_type",
-  trace::Metric::Distribution);
+  "selection_recovery", trace::Metric::Distribution, "language");
+  static constexpr trace::Metric RecoveryType(
+  "selection_recovery_type", trace::Metric::Distribution, "language");
   const auto *Common = S.commonAncestor();
   for (const auto *N = Common; N; N = N->Parent) {
 if (const auto *RE = N->ASTNode.get()) {
-  SelectionUsedRecovery.record(1); // used recovery ast.
-  RecoveryType.record(RE->isTypeDependent() ? 0 : 1);
+  SelectionUsedRecovery.record(1, LanguageLabel); // used recovery ast.
+  RecoveryType.record(RE->isTypeDependent() ? 0 : 1, LanguageLabel);
   return;
 }
   }
   if (Common)
-SelectionUsedRecovery.record(0); // unused.
+SelectionUsedRecovery.record(0, LanguageLabel); // unused.
 }
 
 // An IntervalSet maintains a set of disjoint subranges of an array.
@@ -834,7 +837,7 @@
.printToString(SM));
   Nodes = SelectionVisitor::collect(AST, Tokens, PrintPolicy, Begin, End, FID);
   Root = Nodes.empty() ? nullptr : &Nodes.front();
-  recordMetrics(*this);
+  recordMetrics(*this, AST.getLangOpts());
   dlog("Built selection tree\n{0}", *this);
 }
 


Index: clang-tools-extra/clangd/unittests/SelectionTests.cpp
===
--- clang-tools-extra/clangd/unittests/SelectionTests.cpp
+++ clang-tools-extra/clangd/unittests/SelectionTests.cpp
@@ -18,6 +18,7 @@
 namespace clang {
 namespace clangd {
 namespace {
+using ::testing::ElementsAreArray;
 using ::testing::UnorderedElementsAreArray;
 
 // Create a selection tree corresponding to a point or pair of points.
@@ -452,7 +453,8 @@
 if (Test.ranges().empty()) {
   // If no [[range]] is marked in the example, there should be no s

[PATCH] D92157: [clangd] Add language metrics for recovery AST usage.

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.



Comment at: clang-tools-extra/clangd/Selection.cpp:41
+const char *getLanguage(const clang::LangOptions &Lang) {
+  if (Lang.C99 || Lang.C11 || Lang.C17 || Lang.C2x)
+return "C";

sammccall wrote:
> Testing for specific C versions seems a bit weird to me - what if we're in 
> C89? 
> I'm not sure what the intention is - if the idea is to exclude extensions 
> like openmp I'm not sure this actually does that.
> And by checking it before ObjC I think we're misclassifying `clang -std=c99 
> foo.m`
> 
> I'd suggest checking (optionally objc++), objc, c++, and calling everything 
> else C. If you really want to avoid particular dialects, probably best to 
> name them specifically.
sounds good to me. 



Comment at: clang-tools-extra/clangd/Selection.cpp:65
   RecoveryType.record(RE->isTypeDependent() ? 0 : 1);
+  RecoveryAvailable.record(1, getLanguage(AST.getLangOpts()));
   return;

sammccall wrote:
> I'm not clear what this is trying to measure - why isn't this the same metric 
> as SelectionUsedRecovery (just adding a field to that one?)
yeah, this is a good idea, didn't realize that we could use the label, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92157

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

[PATCH] D92157: [clangd] Add language metrics for recovery AST usage.

[Sam McCall via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/clangd/Selection.cpp:44
+return;
+  const char *LanguageLabel = Lang.CPlusPlus ? "C++" : Lang.ObjC ? "ObjC" : 
"C";
   static constexpr trace::Metric SelectionUsedRecovery(

This classifies objc++ as c++.
Not necessarily a problem, up to you


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92157

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

[PATCH] D90213: [PowerPC] [Clang] Enable float128 feature on P9 by default

[Qiu Chaofan via Phabricator via cfe-commits]

qiucf added a comment.

Ping..


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90213

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

[PATCH] D92408: [clangd] ExtractFunction: disable on regions that sometimes, but not always return.

[Utkarsh Saxena via Phabricator via cfe-commits]

usaxena95 added inline comments.



Comment at: clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp:724
+  ReturnStmtVisitor V;
+  for (const auto *RootStmt : ExtZone.RootStmts) {
+V.TraverseStmt(const_cast(RootStmt));

nit: s/auto/Stmt




Comment at: clang-tools-extra/clangd/unittests/TweakTests.cpp:613
+  EXPECT_THAT(
+  apply("#define RETURN_IF_ERROR(x) if (x) return\nRETU^RN_IF_ERROR(4);"),
+  StartsWith("unavailable"));

usaxena95 wrote:
> super super nit: `!x` instead of x.
Oops this is fine. Ignore this comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92408

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

[PATCH] D92704: [clangd] Publish config file errors over LSP

[Haojian Wu via Phabricator via cfe-commits]

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

The pre-merging bot seems to be failed on windows: 
https://reviews.llvm.org/harbormaster/unit/view/215879/.




Comment at: clang-tools-extra/clangd/ClangdServer.cpp:833
+  llvm::StringMap> ReportableDiagnostics;
+  auto DiagnosticHandler = [&](const llvm::SMDiagnostic &D) {
+// Ensure we create the map entry even for note diagnostics we don't 
report.

I'd encode `Config` into names, `DiagnosticHandler` made me think this is a 
handler for clang diagnostics (without reading the context).



Comment at: clang-tools-extra/clangd/ClangdServer.h:366
   const ThreadsafeFS &TFS;
+  Callbacks *ServerCallbacks;
+  mutable std::mutex ConfigDiagnosticsMu;

nit: = nullptr.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92704

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

[PATCH] D92006: Refactoring the attrubute plugin example to fit the new API

[Yafei Liu via Phabricator via cfe-commits]

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



Comment at: clang/examples/Attribute/Attribute.cpp:81
+D->addAttr(AnnotateAttr::Create(S.Context, "example", &Arg0,
+Attr.getNumArgs(), Attr.getRange()));
 return AttributeApplied;

aaron.ballman wrote:
> This looks dangerous to me -- if there are two or three arguments supplied, 
> then this code will result in a buffer overrun.
Oh, didn't noticed that, but I checked the code it's hard to get the address of 
the argument buffer, all APIs about it is private, even if `ArgsUnion const 
*getArgsBuffer() const`, which I think is not very reasonable. Do you think I 
should copy all the arguments to a new buffer (not very effective), or do you 
think I should contribute a patch to make the API public?

There's a third way, `getTrailingObjects()` is public in TrailingObjects, it is 
possible to cast an `ParsedAttr` to a `TrailingObjects`, but I think it's too 
hacky.



Comment at: clang/test/Frontend/plugin-attribute.cpp:1
-// RUN: %clang -fplugin=%llvmshlibdir/Attribute%pluginext -emit-llvm -S %s -o 
- 2>&1 | FileCheck %s --check-prefix=ATTRIBUTE
-// RUN: not %clang -fplugin=%llvmshlibdir/Attribute%pluginext -emit-llvm 
-DBAD_ATTRIBUTE -S %s -o - 2>&1 | FileCheck %s --check-prefix=BADATTRIBUTE
+// RUN: %clang -fplugin=%llvmshlibdir/Attribute%pluginext -DGOOD_ATTR 
-fsyntax-only -Xclang -verify=goodattr %s
+// RUN: %clang -fplugin=%llvmshlibdir/Attribute%pluginext -DBAD_ATTR 
-fsyntax-only -Xclang -verify=badattr %s

aaron.ballman wrote:
> I don't think we need a GOOD_ATTR and BAD_ATTR run line any longer. Instead, 
> you can use a single RUN line with `-verify` (no `=`) and use 
> `expected-error` and `expected-warning` on the lines that need the 
> diagnostics. This means you can also get rid of the `@ line number` 
> constructs and just write the expected comments on the source lines with 
> diagnostics.
Seems the `GOOD_ATTR` and `BAD_ATTR` need to be exist since we use `-ast-dump` 
now, we need a clean build without error diagnostics to generate a nice AST.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92006

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

[PATCH] D91950: [clang-format] Add BreakBeforeInlineASMColon configuration

[Anastasiia Lukianenko via Phabricator via cfe-commits]

anastasiia_lukianenko added a comment.

This option was created to have an option of removing this break, because 
without my patch it breaks before ASM colon in long lines. That's why I saved 
the behavior of BreakBeforeInlineASMColon=true as it was initially.
Therefore, I'm asking you is this a bug - breaking only long lines or this is 
normal clang-format behavior?


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

https://reviews.llvm.org/D91950

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

[PATCH] D90213: [PowerPC] [Clang] Enable float128 feature on P9 by default

[Qing Shan Zhang via Phabricator via cfe-commits]

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

LGTM as it seems the missing part. Please also fix the error message if with 
Power8.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90213

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

[PATCH] D92677: Provide default location of sysroot for Baremetal toolchain.

[Hafiz Abid Qadeer via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG275592e71413: Provide default location of sysroot for 
Baremetal  toolchain. (authored by abidh).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92677

Files:
  clang/lib/Driver/ToolChains/BareMetal.cpp
  clang/lib/Driver/ToolChains/BareMetal.h
  clang/test/Driver/baremetal-sysroot.cpp


Index: clang/test/Driver/baremetal-sysroot.cpp
===
--- /dev/null
+++ clang/test/Driver/baremetal-sysroot.cpp
@@ -0,0 +1,22 @@
+// REQUIRES: shell
+// UNSUPPORTED: system-windows
+
+// Test that when a --sysroot is not provided, driver picks the default
+// location correctly if available.
+
+// RUN: rm -rf %T/baremetal_default_sysroot
+// RUN: mkdir -p %T/baremetal_default_sysroot/bin
+// RUN: mkdir -p 
%T/baremetal_default_sysroot/lib/clang-runtimes/armv6m-none-eabi
+// RUN: ln -s %clang %T/baremetal_default_sysroot/bin/clang
+
+// RUN: %T/baremetal_default_sysroot/bin/clang -no-canonical-prefixes %s -### 
-o %t.o 2>&1 \
+// RUN: -target armv6m-none-eabi \
+// RUN:   | FileCheck --check-prefix=CHECK-V6M-C %s
+// CHECK-V6M-C: "{{.*}}clang{{.*}}" "-cc1" "-triple" 
"thumbv6m-none-unknown-eabi"
+// CHECK-V6M-C-SAME: "-internal-isystem" 
"{{.*}}/baremetal_default_sysroot{{[/\\]+}}bin{{[/\\]+}}..{{[/\\]+}}lib{{[/\\]+}}clang-runtimes{{[/\\]+}}armv6m-none-eabi{{[/\\]+}}include{{[/\\]+}}c++{{[/\\]+}}v1"
+// CHECk-V6M-C-SAME: "-internal-isystem" 
"{{.*}}/baremetal_default_sysroot{{[/\\]+}}bin{{[/\\]+}}..{{[/\\]+}}lib{{[/\\]+}}clang-runtimes{{[/\\]+}}armv6m-none-eabi{{[/\\]+}}include"
+// CHECK-V6M-C-SAME: "-x" "c++" "{{.*}}baremetal-sysroot.cpp"
+// CHECK-V6M-C-NEXT: "{{[^"]*}}ld{{(\.(lld|bfd|gold))?}}{{(\.exe)?}}" 
"{{.*}}.o" "-Bstatic"
+// CHECK-V6M-C-SAME: 
"-L{{.*}}/baremetal_default_sysroot{{[/\\]+}}bin{{[/\\]+}}..{{[/\\]+}}lib{{[/\\]+}}clang-runtimes{{[/\\]+}}armv6m-none-eabi{{[/\\]+}}lib"
+// CHECK-V6M-C-SAME: "-lc" "-lm" "-lclang_rt.builtins-armv6m"
+// CHECK-V6M-C-SAME: "-o" "{{.*}}.o"
Index: clang/lib/Driver/ToolChains/BareMetal.h
===
--- clang/lib/Driver/ToolChains/BareMetal.h
+++ clang/lib/Driver/ToolChains/BareMetal.h
@@ -67,6 +67,7 @@
llvm::opt::ArgStringList &CmdArgs) const override;
   void AddLinkRuntimeLib(const llvm::opt::ArgList &Args,
  llvm::opt::ArgStringList &CmdArgs) const;
+  std::string computeSysRoot() const override;
 };
 
 } // namespace toolchains
Index: clang/lib/Driver/ToolChains/BareMetal.cpp
===
--- clang/lib/Driver/ToolChains/BareMetal.cpp
+++ clang/lib/Driver/ToolChains/BareMetal.cpp
@@ -33,7 +33,7 @@
   getProgramPaths().push_back(getDriver().getInstalledDir());
   if (getDriver().getInstalledDir() != getDriver().Dir)
 getProgramPaths().push_back(getDriver().Dir);
-  SmallString<128> SysRoot(getDriver().SysRoot);
+  SmallString<128> SysRoot(computeSysRoot());
   if (!SysRoot.empty()) {
 llvm::sys::path::append(SysRoot, "lib");
 getFilePaths().push_back(std::string(SysRoot));
@@ -94,6 +94,17 @@
   return std::string(Dir.str());
 }
 
+std::string BareMetal::computeSysRoot() const {
+  if (!getDriver().SysRoot.empty())
+return getDriver().SysRoot;
+
+  SmallString<128> SysRootDir;
+  llvm::sys::path::append(SysRootDir, getDriver().Dir, "../lib/clang-runtimes",
+  getDriver().getTargetTriple());
+
+  return std::string(SysRootDir);
+}
+
 void BareMetal::AddClangSystemIncludeArgs(const ArgList &DriverArgs,
   ArgStringList &CC1Args) const {
   if (DriverArgs.hasArg(options::OPT_nostdinc))
@@ -106,7 +117,7 @@
   }
 
   if (!DriverArgs.hasArg(options::OPT_nostdlibinc)) {
-SmallString<128> Dir(getDriver().SysRoot);
+SmallString<128> Dir(computeSysRoot());
 if (!Dir.empty()) {
   llvm::sys::path::append(Dir, "include");
   addSystemInclude(DriverArgs, CC1Args, Dir.str());
@@ -127,7 +138,7 @@
   DriverArgs.hasArg(options::OPT_nostdincxx))
 return;
 
-  StringRef SysRoot = getDriver().SysRoot;
+  std::string SysRoot(computeSysRoot());
   if (SysRoot.empty())
 return;
 


Index: clang/test/Driver/baremetal-sysroot.cpp
===
--- /dev/null
+++ clang/test/Driver/baremetal-sysroot.cpp
@@ -0,0 +1,22 @@
+// REQUIRES: shell
+// UNSUPPORTED: system-windows
+
+// Test that when a --sysroot is not provided, driver picks the default
+// location correctly if available.
+
+// RUN: rm -rf %T/baremetal_default_sysroot
+// RUN: mkdir -p %T/baremetal_default_sysroot/bin
+// RUN: mkdir -p %T/baremetal_default_sysroot/lib/clang-runtimes/armv6m-none-eabi
+// RUN: ln -s %clang %T/baremetal_default_sysroot/bin/clang
+
+// RUN: %T/baremeta

[clang] 275592e - Provide default location of sysroot for Baremetal toolchain.

[Hafiz Abid Qadeer via cfe-commits]

Author: Hafiz Abid Qadeer
Date: 2020-12-07T09:19:52Z
New Revision: 275592e714130345a481a5cb889c89b73a98614f

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

LOG: Provide default location of sysroot for Baremetal  toolchain.

Currently, Baremetal toolchain requires user to pass a sysroot location
using a --sysroot flag. This is not very convenient for the user. It also
creates problem for toolchain vendors who don't have a fixed location to
put the sysroot bits.

Clang does provide 'DEFAULT_SYSROOT' which can be used by the toolchain
builder to provide the default location. But it does not work if toolchain
is targeting multiple targets e.g. arm-none-eabi/riscv64-unknown-elf which
clang is capable of doing.

This patch tries to solve this problem by providing a default location of
the toolchain if user does not explicitly provides --sysroot. The exact
location and name can be different but it should fulfill these conditions:

1. The sysroot path should have a target triple element so that multi-target
toolchain problem (as I described above) could be addressed.

2. The location should not be $TOP/$Triple as this is used by gcc generally
and will be a problem for installing both gcc and clang based toolchain at
the same location.

Reviewed By: jroelofs

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

Added: 
clang/test/Driver/baremetal-sysroot.cpp

Modified: 
clang/lib/Driver/ToolChains/BareMetal.cpp
clang/lib/Driver/ToolChains/BareMetal.h

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/BareMetal.cpp 
b/clang/lib/Driver/ToolChains/BareMetal.cpp
index 20ffd4b2cd57..7429c822b7e9 100644
--- a/clang/lib/Driver/ToolChains/BareMetal.cpp
+++ b/clang/lib/Driver/ToolChains/BareMetal.cpp
@@ -33,7 +33,7 @@ BareMetal::BareMetal(const Driver &D, const llvm::Triple 
&Triple,
   getProgramPaths().push_back(getDriver().getInstalledDir());
   if (getDriver().getInstalledDir() != getDriver().Dir)
 getProgramPaths().push_back(getDriver().Dir);
-  SmallString<128> SysRoot(getDriver().SysRoot);
+  SmallString<128> SysRoot(computeSysRoot());
   if (!SysRoot.empty()) {
 llvm::sys::path::append(SysRoot, "lib");
 getFilePaths().push_back(std::string(SysRoot));
@@ -94,6 +94,17 @@ std::string BareMetal::getRuntimesDir() const {
   return std::string(Dir.str());
 }
 
+std::string BareMetal::computeSysRoot() const {
+  if (!getDriver().SysRoot.empty())
+return getDriver().SysRoot;
+
+  SmallString<128> SysRootDir;
+  llvm::sys::path::append(SysRootDir, getDriver().Dir, "../lib/clang-runtimes",
+  getDriver().getTargetTriple());
+
+  return std::string(SysRootDir);
+}
+
 void BareMetal::AddClangSystemIncludeArgs(const ArgList &DriverArgs,
   ArgStringList &CC1Args) const {
   if (DriverArgs.hasArg(options::OPT_nostdinc))
@@ -106,7 +117,7 @@ void BareMetal::AddClangSystemIncludeArgs(const ArgList 
&DriverArgs,
   }
 
   if (!DriverArgs.hasArg(options::OPT_nostdlibinc)) {
-SmallString<128> Dir(getDriver().SysRoot);
+SmallString<128> Dir(computeSysRoot());
 if (!Dir.empty()) {
   llvm::sys::path::append(Dir, "include");
   addSystemInclude(DriverArgs, CC1Args, Dir.str());
@@ -127,7 +138,7 @@ void BareMetal::AddClangCXXStdlibIncludeArgs(
   DriverArgs.hasArg(options::OPT_nostdincxx))
 return;
 
-  StringRef SysRoot = getDriver().SysRoot;
+  std::string SysRoot(computeSysRoot());
   if (SysRoot.empty())
 return;
 

diff  --git a/clang/lib/Driver/ToolChains/BareMetal.h 
b/clang/lib/Driver/ToolChains/BareMetal.h
index 0be9377134ce..3f4fadf8a7c3 100644
--- a/clang/lib/Driver/ToolChains/BareMetal.h
+++ b/clang/lib/Driver/ToolChains/BareMetal.h
@@ -67,6 +67,7 @@ class LLVM_LIBRARY_VISIBILITY BareMetal : public ToolChain {
llvm::opt::ArgStringList &CmdArgs) const override;
   void AddLinkRuntimeLib(const llvm::opt::ArgList &Args,
  llvm::opt::ArgStringList &CmdArgs) const;
+  std::string computeSysRoot() const override;
 };
 
 } // namespace toolchains

diff  --git a/clang/test/Driver/baremetal-sysroot.cpp 
b/clang/test/Driver/baremetal-sysroot.cpp
new file mode 100644
index ..ae174e01417e
--- /dev/null
+++ b/clang/test/Driver/baremetal-sysroot.cpp
@@ -0,0 +1,22 @@
+// REQUIRES: shell
+// UNSUPPORTED: system-windows
+
+// Test that when a --sysroot is not provided, driver picks the default
+// location correctly if available.
+
+// RUN: rm -rf %T/baremetal_default_sysroot
+// RUN: mkdir -p %T/baremetal_default_sysroot/bin
+// RUN: mkdir -p 
%T/baremetal_default_sysroot/lib/clang-runtimes/armv6m-none-eabi
+// RUN: ln -s %clang %T/baremetal_default_sysroot/bin/clang
+
+// RUN: %T/baremetal_default_sysroot/bin/clang -no-canonica

[PATCH] D92157: [clangd] Add language metrics for recovery AST usage.

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.



Comment at: clang-tools-extra/clangd/Selection.cpp:44
+return;
+  const char *LanguageLabel = Lang.CPlusPlus ? "C++" : Lang.ObjC ? "ObjC" : 
"C";
   static constexpr trace::Metric SelectionUsedRecovery(

sammccall wrote:
> This classifies objc++ as c++.
> Not necessarily a problem, up to you
yeah, this is my intention as we don't really care about C++ or ObjC++. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92157

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

[PATCH] D92157: [clangd] Add language metrics for recovery AST usage.

[Haojian Wu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG1df0677e6ac6: [clangd] Add language metrics for recovery AST 
usage. (authored by hokein).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92157

Files:
  clang-tools-extra/clangd/Selection.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
@@ -18,6 +18,7 @@
 namespace clang {
 namespace clangd {
 namespace {
+using ::testing::ElementsAreArray;
 using ::testing::UnorderedElementsAreArray;
 
 // Create a selection tree corresponding to a point or pair of points.
@@ -452,7 +453,8 @@
 if (Test.ranges().empty()) {
   // If no [[range]] is marked in the example, there should be no 
selection.
   EXPECT_FALSE(T.commonAncestor()) << C.Code << "\n" << T;
-  EXPECT_THAT(Tracer.takeMetric("selection_recovery"), testing::IsEmpty());
+  EXPECT_THAT(Tracer.takeMetric("selection_recovery", "C++"),
+  testing::IsEmpty());
 } else {
   // If there is an expected selection, common ancestor should exist
   // with the appropriate node type.
@@ -468,8 +470,8 @@
   // and no nodes outside it are selected.
   EXPECT_TRUE(verifyCommonAncestor(T.root(), T.commonAncestor(), C.Code))
   << C.Code;
-  EXPECT_THAT(Tracer.takeMetric("selection_recovery"),
-  testing::ElementsAreArray({0}));
+  EXPECT_THAT(Tracer.takeMetric("selection_recovery", "C++"),
+  ElementsAreArray({0}));
 }
   }
 }
@@ -494,10 +496,10 @@
   auto AST = TestTU::withCode(Annotations(Code).code()).build();
   trace::TestTracer Tracer;
   auto T = makeSelectionTree(Code, AST);
-  EXPECT_THAT(Tracer.takeMetric("selection_recovery"),
-  testing::ElementsAreArray({1}));
-  EXPECT_THAT(Tracer.takeMetric("selection_recovery_type"),
-  testing::ElementsAreArray({1}));
+  EXPECT_THAT(Tracer.takeMetric("selection_recovery", "C++"),
+  ElementsAreArray({1}));
+  EXPECT_THAT(Tracer.takeMetric("selection_recovery_type", "C++"),
+  ElementsAreArray({1}));
 }
 
 // FIXME: Doesn't select the binary operator node in
Index: clang-tools-extra/clangd/Selection.cpp
===
--- clang-tools-extra/clangd/Selection.cpp
+++ clang-tools-extra/clangd/Selection.cpp
@@ -38,21 +38,24 @@
 using ast_type_traits::DynTypedNode;
 
 // Measure the fraction of selections that were enabled by recovery AST.
-void recordMetrics(const SelectionTree &S) {
+void recordMetrics(const SelectionTree &S, const LangOptions &Lang) {
+  if (!trace::enabled())
+return;
+  const char *LanguageLabel = Lang.CPlusPlus ? "C++" : Lang.ObjC ? "ObjC" : 
"C";
   static constexpr trace::Metric SelectionUsedRecovery(
-  "selection_recovery", trace::Metric::Distribution);
-  static constexpr trace::Metric RecoveryType("selection_recovery_type",
-  trace::Metric::Distribution);
+  "selection_recovery", trace::Metric::Distribution, "language");
+  static constexpr trace::Metric RecoveryType(
+  "selection_recovery_type", trace::Metric::Distribution, "language");
   const auto *Common = S.commonAncestor();
   for (const auto *N = Common; N; N = N->Parent) {
 if (const auto *RE = N->ASTNode.get()) {
-  SelectionUsedRecovery.record(1); // used recovery ast.
-  RecoveryType.record(RE->isTypeDependent() ? 0 : 1);
+  SelectionUsedRecovery.record(1, LanguageLabel); // used recovery ast.
+  RecoveryType.record(RE->isTypeDependent() ? 0 : 1, LanguageLabel);
   return;
 }
   }
   if (Common)
-SelectionUsedRecovery.record(0); // unused.
+SelectionUsedRecovery.record(0, LanguageLabel); // unused.
 }
 
 // An IntervalSet maintains a set of disjoint subranges of an array.
@@ -834,7 +837,7 @@
.printToString(SM));
   Nodes = SelectionVisitor::collect(AST, Tokens, PrintPolicy, Begin, End, FID);
   Root = Nodes.empty() ? nullptr : &Nodes.front();
-  recordMetrics(*this);
+  recordMetrics(*this, AST.getLangOpts());
   dlog("Built selection tree\n{0}", *this);
 }
 


Index: clang-tools-extra/clangd/unittests/SelectionTests.cpp
===
--- clang-tools-extra/clangd/unittests/SelectionTests.cpp
+++ clang-tools-extra/clangd/unittests/SelectionTests.cpp
@@ -18,6 +18,7 @@
 namespace clang {
 namespace clangd {
 namespace {
+using ::testing::ElementsAreArray;
 using ::testing::UnorderedElementsAreArray;
 
 // Create a selection tree corresponding to a point or pair of points.
@@ -452,7 +453,8 @@
 if (Test.ranges().empty()) {
   // If no [[range]]

[clang-tools-extra] 1df0677 - [clangd] Add language metrics for recovery AST usage.

[Haojian Wu via cfe-commits]

Author: Haojian Wu
Date: 2020-12-07T10:52:05+01:00
New Revision: 1df0677e6ac65e18da54b1dd5c391bf17a4c2737

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

LOG: [clangd] Add language metrics for recovery AST usage.

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

Added: 


Modified: 
clang-tools-extra/clangd/Selection.cpp
clang-tools-extra/clangd/unittests/SelectionTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/Selection.cpp 
b/clang-tools-extra/clangd/Selection.cpp
index fbd72be320a7..9f84b4729182 100644
--- a/clang-tools-extra/clangd/Selection.cpp
+++ b/clang-tools-extra/clangd/Selection.cpp
@@ -38,21 +38,24 @@ using Node = SelectionTree::Node;
 using ast_type_traits::DynTypedNode;
 
 // Measure the fraction of selections that were enabled by recovery AST.
-void recordMetrics(const SelectionTree &S) {
+void recordMetrics(const SelectionTree &S, const LangOptions &Lang) {
+  if (!trace::enabled())
+return;
+  const char *LanguageLabel = Lang.CPlusPlus ? "C++" : Lang.ObjC ? "ObjC" : 
"C";
   static constexpr trace::Metric SelectionUsedRecovery(
-  "selection_recovery", trace::Metric::Distribution);
-  static constexpr trace::Metric RecoveryType("selection_recovery_type",
-  trace::Metric::Distribution);
+  "selection_recovery", trace::Metric::Distribution, "language");
+  static constexpr trace::Metric RecoveryType(
+  "selection_recovery_type", trace::Metric::Distribution, "language");
   const auto *Common = S.commonAncestor();
   for (const auto *N = Common; N; N = N->Parent) {
 if (const auto *RE = N->ASTNode.get()) {
-  SelectionUsedRecovery.record(1); // used recovery ast.
-  RecoveryType.record(RE->isTypeDependent() ? 0 : 1);
+  SelectionUsedRecovery.record(1, LanguageLabel); // used recovery ast.
+  RecoveryType.record(RE->isTypeDependent() ? 0 : 1, LanguageLabel);
   return;
 }
   }
   if (Common)
-SelectionUsedRecovery.record(0); // unused.
+SelectionUsedRecovery.record(0, LanguageLabel); // unused.
 }
 
 // An IntervalSet maintains a set of disjoint subranges of an array.
@@ -834,7 +837,7 @@ SelectionTree::SelectionTree(ASTContext &AST, const 
syntax::TokenBuffer &Tokens,
.printToString(SM));
   Nodes = SelectionVisitor::collect(AST, Tokens, PrintPolicy, Begin, End, FID);
   Root = Nodes.empty() ? nullptr : &Nodes.front();
-  recordMetrics(*this);
+  recordMetrics(*this, AST.getLangOpts());
   dlog("Built selection tree\n{0}", *this);
 }
 

diff  --git a/clang-tools-extra/clangd/unittests/SelectionTests.cpp 
b/clang-tools-extra/clangd/unittests/SelectionTests.cpp
index 55b05ebe11ab..efe06383b04a 100644
--- a/clang-tools-extra/clangd/unittests/SelectionTests.cpp
+++ b/clang-tools-extra/clangd/unittests/SelectionTests.cpp
@@ -18,6 +18,7 @@
 namespace clang {
 namespace clangd {
 namespace {
+using ::testing::ElementsAreArray;
 using ::testing::UnorderedElementsAreArray;
 
 // Create a selection tree corresponding to a point or pair of points.
@@ -452,7 +453,8 @@ TEST(SelectionTest, CommonAncestor) {
 if (Test.ranges().empty()) {
   // If no [[range]] is marked in the example, there should be no 
selection.
   EXPECT_FALSE(T.commonAncestor()) << C.Code << "\n" << T;
-  EXPECT_THAT(Tracer.takeMetric("selection_recovery"), testing::IsEmpty());
+  EXPECT_THAT(Tracer.takeMetric("selection_recovery", "C++"),
+  testing::IsEmpty());
 } else {
   // If there is an expected selection, common ancestor should exist
   // with the appropriate node type.
@@ -468,8 +470,8 @@ TEST(SelectionTest, CommonAncestor) {
   // and no nodes outside it are selected.
   EXPECT_TRUE(verifyCommonAncestor(T.root(), T.commonAncestor(), C.Code))
   << C.Code;
-  EXPECT_THAT(Tracer.takeMetric("selection_recovery"),
-  testing::ElementsAreArray({0}));
+  EXPECT_THAT(Tracer.takeMetric("selection_recovery", "C++"),
+  ElementsAreArray({0}));
 }
   }
 }
@@ -494,10 +496,10 @@ TEST(SelectionTree, Metrics) {
   auto AST = TestTU::withCode(Annotations(Code).code()).build();
   trace::TestTracer Tracer;
   auto T = makeSelectionTree(Code, AST);
-  EXPECT_THAT(Tracer.takeMetric("selection_recovery"),
-  testing::ElementsAreArray({1}));
-  EXPECT_THAT(Tracer.takeMetric("selection_recovery_type"),
-  testing::ElementsAreArray({1}));
+  EXPECT_THAT(Tracer.takeMetric("selection_recovery", "C++"),
+  ElementsAreArray({1}));
+  EXPECT_THAT(Tracer.takeMetric("selection_recovery_type", "C++"),
+  ElementsAreArray({1}));
 }
 
 // FIXME: Doesn't select the binary operator node in



__

[PATCH] D92704: [clangd] Publish config file errors over LSP

[Sam McCall via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
sammccall marked 2 inline comments as done.
Closed by commit rGfed9af29c2b5: [clangd] Publish config file errors over LSP 
(authored by sammccall).

Changed prior to commit:
  https://reviews.llvm.org/D92704?vs=309688&id=309841#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92704

Files:
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/ConfigProvider.h
  clang-tools-extra/clangd/ConfigYAML.cpp
  clang-tools-extra/clangd/Diagnostics.cpp
  clang-tools-extra/clangd/Diagnostics.h
  clang-tools-extra/clangd/test/config.test
  clang-tools-extra/clangd/unittests/ConfigProviderTests.cpp
  clang-tools-extra/clangd/unittests/ConfigTesting.h
  clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp

Index: clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp
===
--- clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp
+++ clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp
@@ -67,6 +67,7 @@
   )yaml";
   auto Results = Fragment::parseYAML(YAML, "config.yaml", Diags.callback());
   EXPECT_THAT(Diags.Diagnostics, IsEmpty());
+  EXPECT_THAT(Diags.Files, ElementsAre("config.yaml"));
   ASSERT_EQ(Results.size(), 4u);
   EXPECT_FALSE(Results[0].If.HasUnrecognizedCondition);
   EXPECT_THAT(Results[0].If.PathMatch, ElementsAre(Val("abc")));
Index: clang-tools-extra/clangd/unittests/ConfigTesting.h
===
--- clang-tools-extra/clangd/unittests/ConfigTesting.h
+++ clang-tools-extra/clangd/unittests/ConfigTesting.h
@@ -24,6 +24,12 @@
 struct CapturedDiags {
   std::function callback() {
 return [this](const llvm::SMDiagnostic &D) {
+  if (Files.empty() || Files.back() != D.getFilename())
+Files.push_back(D.getFilename().str());
+
+  if (D.getKind() > llvm::SourceMgr::DK_Warning)
+return;
+
   Diagnostics.emplace_back();
   Diag &Out = Diagnostics.back();
   Out.Message = D.getMessage().str();
@@ -50,7 +56,13 @@
   << D.Message << "@" << llvm::to_string(D.Pos);
 }
   };
-  std::vector Diagnostics;
+  std::vector Diagnostics;  // Warning or higher.
+  std::vector Files; // Filename from diagnostics including notes.
+
+  void clear() {
+Diagnostics.clear();
+Files.clear();
+  }
 };
 
 MATCHER_P(DiagMessage, M, "") { return arg.Message == M; }
Index: clang-tools-extra/clangd/unittests/ConfigProviderTests.cpp
===
--- clang-tools-extra/clangd/unittests/ConfigProviderTests.cpp
+++ clang-tools-extra/clangd/unittests/ConfigProviderTests.cpp
@@ -96,21 +96,26 @@
   auto Cfg = P->getConfig(Params(), Diags.callback());
   EXPECT_THAT(Diags.Diagnostics,
   ElementsAre(DiagMessage("Unknown CompileFlags key Unknown")));
+  EXPECT_THAT(Diags.Files, ElementsAre(testPath("foo.yaml")));
   EXPECT_THAT(getAddedArgs(Cfg), ElementsAre("foo"));
-  Diags.Diagnostics.clear();
+  Diags.clear();
 
   Cfg = P->getConfig(Params(), Diags.callback());
   EXPECT_THAT(Diags.Diagnostics, IsEmpty()) << "Cached, not re-parsed";
+  EXPECT_THAT(Diags.Files, IsEmpty());
   EXPECT_THAT(getAddedArgs(Cfg), ElementsAre("foo"));
 
   FS.Files["foo.yaml"] = AddBarBaz;
   Cfg = P->getConfig(Params(), Diags.callback());
   EXPECT_THAT(Diags.Diagnostics, IsEmpty()) << "New config, no errors";
+  EXPECT_THAT(Diags.Files, ElementsAre(testPath("foo.yaml")));
   EXPECT_THAT(getAddedArgs(Cfg), ElementsAre("bar", "baz"));
+  Diags.clear();
 
   FS.Files.erase("foo.yaml");
   Cfg = P->getConfig(Params(), Diags.callback());
   EXPECT_THAT(Diags.Diagnostics, IsEmpty()) << "Missing file is not an error";
+  EXPECT_THAT(Diags.Files, IsEmpty());
   EXPECT_THAT(getAddedArgs(Cfg), IsEmpty());
 }
 
@@ -133,21 +138,26 @@
 
   auto Cfg = P->getConfig(Params(), Diags.callback());
   EXPECT_THAT(Diags.Diagnostics, IsEmpty());
+  EXPECT_THAT(Diags.Files, IsEmpty());
   EXPECT_THAT(getAddedArgs(Cfg), IsEmpty());
 
   Cfg = P->getConfig(ABCParams, Diags.callback());
   EXPECT_THAT(Diags.Diagnostics,
   ElementsAre(DiagMessage("Unknown CompileFlags key Unknown")));
+  EXPECT_THAT(Diags.Files,
+  ElementsAre(testPath("a/foo.yaml"), testPath("a/b/c/foo.yaml")));
   EXPECT_THAT(getAddedArgs(Cfg), ElementsAre("foo", "bar", "baz"));
-  Diags.Diagnostics.clear();
+  Diags.clear();
 
   Cfg = P->getConfig(AParams, Diags.callback());
   EXPECT_THAT(Diags.Diagnostics, IsEmpty()) << "Cached config";
+  EXPECT_THAT(Diags.Files, IsEmpty());
   EXPECT_THAT(getAddedArgs(Cfg), ElementsAre("foo"));
 
   FS.Files.erase("a/foo.yaml");
   Cfg = P->getConfig(ABCParams, Diags.callback());
   EXPECT_THAT(Diags.Diagnostics, IsEmpty());
+  EXPECT_THAT(Diags.F

[clang-tools-extra] fed9af2 - [clangd] Publish config file errors over LSP

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2020-12-07T11:07:32+01:00
New Revision: fed9af29c2b5f289744254390c7372e8871e45e5

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

LOG: [clangd] Publish config file errors over LSP

We don't act as a language server for these files (e.g. don't get open/close
notifications for them), but just blindly publish diagnostics for them.

This works reasonably well in coc.nvim and vscode: they show up in the
workspace diagnostic list and when you open the file.
The only update after the file is reparsed, not as you type which is a bit
janky, but seems a lot better than nothing.

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

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

Added: 
clang-tools-extra/clangd/test/config.test

Modified: 
clang-tools-extra/clangd/ClangdServer.cpp
clang-tools-extra/clangd/ClangdServer.h
clang-tools-extra/clangd/ConfigProvider.h
clang-tools-extra/clangd/ConfigYAML.cpp
clang-tools-extra/clangd/Diagnostics.cpp
clang-tools-extra/clangd/Diagnostics.h
clang-tools-extra/clangd/unittests/ConfigProviderTests.cpp
clang-tools-extra/clangd/unittests/ConfigTesting.h
clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/ClangdServer.cpp 
b/clang-tools-extra/clangd/ClangdServer.cpp
index aa19a9485e17..8b0d0591abe7 100644
--- a/clang-tools-extra/clangd/ClangdServer.cpp
+++ b/clang-tools-extra/clangd/ClangdServer.cpp
@@ -139,7 +139,7 @@ ClangdServer::Options::operator TUScheduler::Options() 
const {
 ClangdServer::ClangdServer(const GlobalCompilationDatabase &CDB,
const ThreadsafeFS &TFS, const Options &Opts,
Callbacks *Callbacks)
-: ConfigProvider(Opts.ConfigProvider), TFS(TFS),
+: ConfigProvider(Opts.ConfigProvider), TFS(TFS), 
ServerCallbacks(Callbacks),
   DynamicIdx(Opts.BuildDynamicSymbolIndex
  ? new FileIndex(Opts.HeavyweightDynamicSymbolIndex,
  Opts.CollectMainFileRefs)
@@ -829,16 +829,44 @@ Context ClangdServer::createProcessingContext(PathRef 
File) const {
 Params.Path = PosixPath.str();
   }
 
-  auto DiagnosticHandler = [](const llvm::SMDiagnostic &Diag) {
-if (Diag.getKind() == llvm::SourceMgr::DK_Error) {
-  elog("config error at {0}:{1}:{2}: {3}", Diag.getFilename(),
-   Diag.getLineNo(), Diag.getColumnNo(), Diag.getMessage());
-} else {
-  log("config warning at {0}:{1}:{2}: {3}", Diag.getFilename(),
-  Diag.getLineNo(), Diag.getColumnNo(), Diag.getMessage());
+  llvm::StringMap> ReportableDiagnostics;
+  auto ConfigDiagnosticHandler = [&](const llvm::SMDiagnostic &D) {
+// Ensure we create the map entry even for note diagnostics we don't 
report.
+// This means that when the file is parsed with no warnings, we'll
+// publish an empty set of diagnostics, clearing any the client has.
+auto *Reportable = D.getFilename().empty()
+   ? nullptr
+   : &ReportableDiagnostics[D.getFilename()];
+switch (D.getKind()) {
+case llvm::SourceMgr::DK_Error:
+  elog("config error at {0}:{1}:{2}: {3}", D.getFilename(), D.getLineNo(),
+   D.getColumnNo(), D.getMessage());
+  if (Reportable)
+Reportable->push_back(toDiag(D, Diag::ClangdConfig));
+  break;
+case llvm::SourceMgr::DK_Warning:
+  log("config warning at {0}:{1}:{2}: {3}", D.getFilename(), D.getLineNo(),
+  D.getColumnNo(), D.getMessage());
+  if (Reportable)
+Reportable->push_back(toDiag(D, Diag::ClangdConfig));
+  break;
+case llvm::SourceMgr::DK_Note:
+case llvm::SourceMgr::DK_Remark:
+  vlog("config note at {0}:{1}:{2}: {3}", D.getFilename(), D.getLineNo(),
+   D.getColumnNo(), D.getMessage());
+  break;
 }
   };
-  Config C = ConfigProvider->getConfig(Params, DiagnosticHandler);
+  Config C = ConfigProvider->getConfig(Params, ConfigDiagnosticHandler);
+  // Blindly publish diagnostics for the (unopened) parsed config files.
+  // We must avoid reporting diagnostics for *the same file* concurrently.
+  // Source file diags are published elsewhere, but those are 
diff erent files.
+  if (!ReportableDiagnostics.empty()) {
+std::lock_guard Lock(ConfigDiagnosticsMu);
+for (auto &Entry : ReportableDiagnostics)
+  ServerCallbacks->onDiagnosticsReady(Entry.first(), /*Version=*/"",
+  std::move(Entry.second));
+  }
   return Context::current().derive(Config::Key, std::move(C));
 }
 

diff  --git a/clang-tools-extra/clangd/ClangdServer.h 
b/clang-tools-extra/clangd/ClangdServer.h
index 35ba4686cc9a..ff2fc8578103 100644
--- a/clang-tool

[PATCH] D92749: [clangd] go-to-implementation on a base class jumps to all subclasses.

[Haojian Wu via Phabricator via cfe-commits]

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

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D92749

Files:
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/XRefs.h
  clang-tools-extra/clangd/unittests/XRefsTests.cpp

Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -1483,11 +1483,11 @@
 
 TEST(FindImplementations, Inheritance) {
   llvm::StringRef Test = R"cpp(
-struct Base {
+struct $0^Base {
   virtual void F$1^oo();
   void C$4^oncrete();
 };
-struct Child1 : Base {
+struct $0[[Child1]] : Base {
   void $1[[Fo$3^o]]() override;
   virtual void B$2^ar();
   void Concrete();  // No implementations for concrete methods.
@@ -1497,7 +1497,7 @@
   void $2[[Bar]]() override;
 };
 void FromReference() {
-  Base* B;
+  $0^Base* B;
   B->Fo$1^o();
   B->C$4^oncrete();
   &Base::Fo$1^o;
@@ -1510,7 +1510,7 @@
   Annotations Code(Test);
   auto TU = TestTU::withCode(Code.code());
   auto AST = TU.build();
-  for (const std::string &Label : {"1", "2", "3", "4"}) {
+  for (const std::string &Label : {"0", "1", "2", "3", "4"}) {
 for (const auto &Point : Code.points(Label)) {
   EXPECT_THAT(findImplementations(AST, Point, TU.index().get()),
   UnorderedPointwise(DeclRange(), Code.ranges(Label)))
Index: clang-tools-extra/clangd/XRefs.h
===
--- clang-tools-extra/clangd/XRefs.h
+++ clang-tools-extra/clangd/XRefs.h
@@ -83,7 +83,9 @@
   bool HasMore = false;
 };
 
-/// Returns implementations of the virtual function at a specified \p Pos.
+/// Returns implementations at a specified \p Pos:
+///   - overrides for a virtual method;
+///   - subclasses for a base class;
 std::vector findImplementations(ParsedAST &AST, Position Pos,
const SymbolIndex *Index);
 
Index: clang-tools-extra/clangd/XRefs.cpp
===
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -292,13 +292,14 @@
   return D;
 }
 
-std::vector findOverrides(llvm::DenseSet IDs,
- const SymbolIndex *Index,
- llvm::StringRef MainFilePath) {
+std::vector findImplementors(llvm::DenseSet IDs,
+RelationKind Predicate,
+const SymbolIndex *Index,
+llvm::StringRef MainFilePath) {
   if (IDs.empty())
 return {};
   RelationsRequest Req;
-  Req.Predicate = RelationKind::OverriddenBy;
+  Req.Predicate = Predicate;
   Req.Subjects = std::move(IDs);
   std::vector Results;
   Index->relations(Req, [&](const SymbolID &Subject, const Symbol &Object) {
@@ -458,7 +459,8 @@
 });
   }
 
-  auto Overrides = findOverrides(VirtualMethods, Index, MainFilePath);
+  auto Overrides = findImplementors(VirtualMethods, RelationKind::OverriddenBy,
+Index, MainFilePath);
   Result.insert(Result.end(), Overrides.begin(), Overrides.end());
   return Result;
 }
@@ -1187,12 +1189,20 @@
   }
   DeclRelationSet Relations =
   DeclRelation::TemplatePattern | DeclRelation::Alias;
-  llvm::DenseSet VirtualMethods;
-  for (const NamedDecl *ND : getDeclAtPosition(AST, *CurLoc, Relations))
-if (const CXXMethodDecl *CXXMD = llvm::dyn_cast(ND))
-  if (CXXMD->isVirtual())
-VirtualMethods.insert(getSymbolID(ND));
-  return findOverrides(std::move(VirtualMethods), Index, *MainFilePath);
+  llvm::DenseSet IDs;
+  RelationKind QueryKind;
+  for (const NamedDecl *ND : getDeclAtPosition(AST, *CurLoc, Relations)) {
+if (const auto *CXXMD = llvm::dyn_cast(ND))
+  if (CXXMD->isVirtual()) {
+IDs.insert(getSymbolID(ND));
+QueryKind = RelationKind::OverriddenBy;
+  }
+if (const auto *D = dyn_cast(ND)) {
+  IDs.insert(getSymbolID(D));
+  QueryKind = RelationKind::BaseOf;
+}
+  }
+  return findImplementors(std::move(IDs), QueryKind, Index, *MainFilePath);
 }
 
 ReferencesResult findReferences(ParsedAST &AST, Position Pos, uint32_t Limit,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D49890: Clang-Tidy Export Problem

[Johnny Willemsen via Phabricator via cfe-commits]

jwillemsen added a comment.

I am trying to use run-clang-tidy on ACE/TAO (see 
https://github.com/DOCGroup/ACE_TAO) but it doesn't work, also the error 
reported here. When only running modernize-use-override I get a yaml file of 
110167 lines but that can't also be applied using clang-apply-replacements, 
same errors.

The last proposed change to run-clang-tidy.py doesn't work for me.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D49890

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

[PATCH] D90392: [clang-tidy] Omit std::make_unique/make_shared for default initialization.

[Haojian Wu via Phabricator via cfe-commits]

hokein accepted this revision.
hokein added a comment.

thanks, I think this patch is good, feel free to land it (and sorry for the 
delay).

As @lebedev.ri commented, would be nice to mention the motivation of the change 
(`make_shared_for_overwrite`?) in the description.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90392

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

[PATCH] D90213: [PowerPC] [Clang] Enable float128 feature on P9 by default

[Qiu Chaofan 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 rG6bf29dbb1541: [PowerPC] [Clang] Enable float128 feature on 
P9 by default (authored by qiucf).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90213

Files:
  clang/lib/Basic/Targets/PPC.cpp
  clang/test/Driver/ppc-f128-support-check.c


Index: clang/test/Driver/ppc-f128-support-check.c
===
--- clang/test/Driver/ppc-f128-support-check.c
+++ clang/test/Driver/ppc-f128-support-check.c
@@ -1,7 +1,7 @@
 // RUN: not %clang -target powerpc64le-unknown-linux-gnu -fsyntax-only \
-// RUN:   -mcpu=pwr9 -mfloat128 %s 2>&1 | FileCheck %s --check-prefix=HASF128
+// RUN:   -mcpu=pwr9 %s 2>&1 | FileCheck %s --check-prefix=HASF128
 // RUN: not %clang -target powerpc64le-unknown-linux-gnu -fsyntax-only \
-// RUN:   -mcpu=power9 -mfloat128 %s 2>&1 | FileCheck %s --check-prefix=HASF128
+// RUN:   -mcpu=power9 %s 2>&1 | FileCheck %s --check-prefix=HASF128
 
 // RUN: not %clang -target powerpc64le-unknown-linux-gnu -fsyntax-only \
 // RUN:   -mcpu=pwr8 -mfloat128 %s 2>&1 | FileCheck %s --check-prefix=NOF128
Index: clang/lib/Basic/Targets/PPC.cpp
===
--- clang/lib/Basic/Targets/PPC.cpp
+++ clang/lib/Basic/Targets/PPC.cpp
@@ -317,6 +317,9 @@
 .Case("pwr9", true)
 .Case("pwr8", true)
 .Default(false);
+  Features["float128"] = llvm::StringSwitch(CPU)
+.Case("pwr9", true)
+.Default(false);
 
   Features["spe"] = llvm::StringSwitch(CPU)
 .Case("8548", true)


Index: clang/test/Driver/ppc-f128-support-check.c
===
--- clang/test/Driver/ppc-f128-support-check.c
+++ clang/test/Driver/ppc-f128-support-check.c
@@ -1,7 +1,7 @@
 // RUN: not %clang -target powerpc64le-unknown-linux-gnu -fsyntax-only \
-// RUN:   -mcpu=pwr9 -mfloat128 %s 2>&1 | FileCheck %s --check-prefix=HASF128
+// RUN:   -mcpu=pwr9 %s 2>&1 | FileCheck %s --check-prefix=HASF128
 // RUN: not %clang -target powerpc64le-unknown-linux-gnu -fsyntax-only \
-// RUN:   -mcpu=power9 -mfloat128 %s 2>&1 | FileCheck %s --check-prefix=HASF128
+// RUN:   -mcpu=power9 %s 2>&1 | FileCheck %s --check-prefix=HASF128
 
 // RUN: not %clang -target powerpc64le-unknown-linux-gnu -fsyntax-only \
 // RUN:   -mcpu=pwr8 -mfloat128 %s 2>&1 | FileCheck %s --check-prefix=NOF128
Index: clang/lib/Basic/Targets/PPC.cpp
===
--- clang/lib/Basic/Targets/PPC.cpp
+++ clang/lib/Basic/Targets/PPC.cpp
@@ -317,6 +317,9 @@
 .Case("pwr9", true)
 .Case("pwr8", true)
 .Default(false);
+  Features["float128"] = llvm::StringSwitch(CPU)
+.Case("pwr9", true)
+.Default(false);
 
   Features["spe"] = llvm::StringSwitch(CPU)
 .Case("8548", true)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D92749: [clangd] go-to-implementation on a base class jumps to all subclasses.

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

How does this handle Dependent base classes (including CRTP). Can tests be 
added to demonstrate that behaviour if its supported.




Comment at: clang-tools-extra/clangd/XRefs.cpp:1200
+  }
+if (const auto *D = dyn_cast(ND)) {
+  IDs.insert(getSymbolID(D));

nit: Make this `else if` or add a continue statement to the the previous if. We 
know ND can't be a `CXXRecordDecl` its a `CXXMethodDecl`.
super-nit: Maybe call this `CXXRD` or just `RD`, `D` implies we only know its a 
`Decl` but nothing more.



Comment at: clang-tools-extra/clangd/unittests/XRefsTests.cpp:1513
   auto AST = TU.build();
-  for (const std::string &Label : {"1", "2", "3", "4"}) {
+  for (const std::string &Label : {"0", "1", "2", "3", "4"}) {
 for (const auto &Point : Code.points(Label)) {

If you're changing this line, may as well change use `StringRef` here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92749

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

[clang] 6bf29db - [PowerPC] [Clang] Enable float128 feature on P9 by default

[Qiu Chaofan via cfe-commits]

Author: Qiu Chaofan
Date: 2020-12-07T18:31:00+08:00
New Revision: 6bf29dbb1541aff717e52b5c5fb12b84f5b38f21

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

LOG: [PowerPC] [Clang] Enable float128 feature on P9 by default

As Power9 introduced hardware support for IEEE quad-precision FP type,
the feature should be enabled by default on Power9 or newer targets.

Reviewed By: steven.zhang

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

Added: 


Modified: 
clang/lib/Basic/Targets/PPC.cpp
clang/test/Driver/ppc-f128-support-check.c

Removed: 




diff  --git a/clang/lib/Basic/Targets/PPC.cpp b/clang/lib/Basic/Targets/PPC.cpp
index 7f6f3d7e0c9f..a6997324acf9 100644
--- a/clang/lib/Basic/Targets/PPC.cpp
+++ b/clang/lib/Basic/Targets/PPC.cpp
@@ -317,6 +317,9 @@ bool PPCTargetInfo::initFeatureMap(
 .Case("pwr9", true)
 .Case("pwr8", true)
 .Default(false);
+  Features["float128"] = llvm::StringSwitch(CPU)
+.Case("pwr9", true)
+.Default(false);
 
   Features["spe"] = llvm::StringSwitch(CPU)
 .Case("8548", true)

diff  --git a/clang/test/Driver/ppc-f128-support-check.c 
b/clang/test/Driver/ppc-f128-support-check.c
index 24748905612f..2e4b7a7ae09c 100644
--- a/clang/test/Driver/ppc-f128-support-check.c
+++ b/clang/test/Driver/ppc-f128-support-check.c
@@ -1,7 +1,7 @@
 // RUN: not %clang -target powerpc64le-unknown-linux-gnu -fsyntax-only \
-// RUN:   -mcpu=pwr9 -mfloat128 %s 2>&1 | FileCheck %s --check-prefix=HASF128
+// RUN:   -mcpu=pwr9 %s 2>&1 | FileCheck %s --check-prefix=HASF128
 // RUN: not %clang -target powerpc64le-unknown-linux-gnu -fsyntax-only \
-// RUN:   -mcpu=power9 -mfloat128 %s 2>&1 | FileCheck %s --check-prefix=HASF128
+// RUN:   -mcpu=power9 %s 2>&1 | FileCheck %s --check-prefix=HASF128
 
 // RUN: not %clang -target powerpc64le-unknown-linux-gnu -fsyntax-only \
 // RUN:   -mcpu=pwr8 -mfloat128 %s 2>&1 | FileCheck %s --check-prefix=NOF128



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

[PATCH] D92006: Refactoring the attrubute plugin example to fit the new API

[Yafei Liu via Phabricator via cfe-commits]

psionic12 updated this revision to Diff 309853.
psionic12 marked 2 inline comments as done.
psionic12 added a comment.

Use a vector to collect arguments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92006

Files:
  clang/examples/Attribute/Attribute.cpp
  clang/test/Frontend/plugin-attribute.cpp

Index: clang/test/Frontend/plugin-attribute.cpp
===
--- clang/test/Frontend/plugin-attribute.cpp
+++ clang/test/Frontend/plugin-attribute.cpp
@@ -1,25 +1,23 @@
-// RUN: %clang -fplugin=%llvmshlibdir/Attribute%pluginext -emit-llvm -S %s -o - 2>&1 | FileCheck %s --check-prefix=ATTRIBUTE
-// RUN: not %clang -fplugin=%llvmshlibdir/Attribute%pluginext -emit-llvm -DBAD_ATTRIBUTE -S %s -o - 2>&1 | FileCheck %s --check-prefix=BADATTRIBUTE
+// RUN: %clang -cc1 -load %llvmshlibdir/Attribute%pluginext -DGOOD_ATTR -fsyntax-only -ast-dump -verify=goodattr %s | FileCheck %s
+// RUN: %clang -fplugin=%llvmshlibdir/Attribute%pluginext -DBAD_ATTR -fsyntax-only -Xclang -verify=badattr %s
 // REQUIRES: plugins, examples
+#ifdef GOOD_ATTR
+// goodattr-no-diagnostics
+void fn1a() __attribute__((example)) {}
+[[example]] void fn1b() {}
+[[plugin::example]] void fn1c() {}
+void fn2() __attribute__((example("somestring", 1, 2.0))) {}
+// CHECK-COUNT-4: -AnnotateAttr 0x{{[0-9a-z]+}} {{}} "example"
+// CHECK: -StringLiteral 0x{{[0-9a-z]+}} {{}} 'const char [{{[0-9]+}}]' lvalue "somestring"
+// CHECK: -IntegerLiteral 0x{{[0-9a-z]+}} {{}} 'int' 1
+// CHECK: -FloatingLiteral 0x{{[0-9a-z]+}} {{}} 'double' 2.00e+00
+#endif
 
-void fn1a() __attribute__((example)) { }
-[[example]] void fn1b() { }
-[[plugin::example]] void fn1c() { }
-void fn2() __attribute__((example("somestring"))) { }
-// ATTRIBUTE: warning: 'example' attribute only applies to functions
-int var1 __attribute__((example("otherstring"))) = 1;
-
-// ATTRIBUTE: [[STR1_VAR:@.+]] = private unnamed_addr constant [10 x i8] c"example()\00"
-// ATTRIBUTE: [[STR2_VAR:@.+]] = private unnamed_addr constant [20 x i8] c"example(somestring)\00"
-// ATTRIBUTE: @llvm.global.annotations = {{.*}}@{{.*}}fn1a{{.*}}[[STR1_VAR]]{{.*}}@{{.*}}fn1b{{.*}}[[STR1_VAR]]{{.*}}@{{.*}}fn1c{{.*}}[[STR1_VAR]]{{.*}}@{{.*}}fn2{{.*}}[[STR2_VAR]]
-
-#ifdef BAD_ATTRIBUTE
+#ifdef BAD_ATTR
+int var1 __attribute__((example("otherstring"))) = 1; // badattr-warning {{'example' attribute only applies to functions}}
 class Example {
-  // BADATTRIBUTE: error: 'example' attribute only allowed at file scope
-  void __attribute__((example)) fn3();
+  void __attribute__((example)) fn3(); // badattr-error {{'example' attribute only allowed at file scope}}
 };
-// BADATTRIBUTE: error: 'example' attribute requires a string
-void fn4() __attribute__((example(123))) { }
-// BADATTRIBUTE: error: 'example' attribute takes no more than 1 argument
-void fn5() __attribute__((example("a","b"))) { }
+void fn4() __attribute__((example(123))) { } // badattr-error {{'example's first argument should be a string literal}}
+void fn5() __attribute__((example("a","b", 3, 4.0))) { } // badattr-error {{'example' attribute only allowed at most three arguments}}
 #endif
Index: clang/examples/Attribute/Attribute.cpp
===
--- clang/examples/Attribute/Attribute.cpp
+++ clang/examples/Attribute/Attribute.cpp
@@ -23,9 +23,10 @@
 
 struct ExampleAttrInfo : public ParsedAttrInfo {
   ExampleAttrInfo() {
-// Can take an optional string argument (the check that the argument
-// actually is a string happens in handleDeclAttribute).
-OptArgs = 1;
+// Can take up to 15 optional arguments, to emulate accepting a variadic
+// number of arguments. This just illustrates how many arguments a
+// `ParsedAttrInfo` can hold, we will not use that much in this example.
+OptArgs = 15;
 // GNU-style __attribute__(("example")) and C++-style [[example]] and
 // [[plugin::example]] supported.
 static constexpr Spelling S[] = {{ParsedAttr::AS_GNU, "example"},
@@ -39,7 +40,7 @@
 // This attribute appertains to functions only.
 if (!isa(D)) {
   S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type_str)
-<< Attr << "functions";
+  << Attr << "functions";
   return false;
 }
 return true;
@@ -55,23 +56,39 @@
   S.Diag(Attr.getLoc(), ID);
   return AttributeNotApplied;
 }
-// Check if we have an optional string argument.
-StringRef Str = "";
+// We make some rules here:
+// 1. Only accept at most 3 arguments here.
+// 2. The first argument must be a string literal if it exists.
+if (Attr.getNumArgs() > 3) {
+  unsigned ID = S.getDiagnostics().getCustomDiagID(
+  DiagnosticsEngine::Error,
+  "'example' attribute only allowed at most three arguments");
+  S.Diag(Attr.getLoc(), ID);
+  return AttributeNotApplied;
+}
+/

[PATCH] D92663: [clangd] Add hot-reload of compile_commands.json and compile_flags.txt

[Nathan James via Phabricator via cfe-commits]

njames93 added inline comments.



Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:712
+  //  - .clang_format and .clang-tidy
+  //  - compile_commands.json
 }

Any reason for re-specifying compile_commands.json here?



Comment at: clang-tools-extra/clangd/GlobalCompilationDatabase.cpp:241
+// thread-safety guarantees, as the access to FS is not locked!
+// For now, use the real FS, which known threadsafe (if we don't use/change
+// working directory, which ExpandResponseFilesDatabase does not).




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92663

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

[PATCH] D92006: Refactoring the attrubute plugin example to fit the new API

[Yafei Liu via Phabricator via cfe-commits]

psionic12 added inline comments.



Comment at: clang/examples/Attribute/Attribute.cpp:81
+D->addAttr(AnnotateAttr::Create(S.Context, "example", &Arg0,
+Attr.getNumArgs(), Attr.getRange()));
 return AttributeApplied;

psionic12 wrote:
> aaron.ballman wrote:
> > This looks dangerous to me -- if there are two or three arguments supplied, 
> > then this code will result in a buffer overrun.
> Oh, didn't noticed that, but I checked the code it's hard to get the address 
> of the argument buffer, all APIs about it is private, even if `ArgsUnion 
> const *getArgsBuffer() const`, which I think is not very reasonable. Do you 
> think I should copy all the arguments to a new buffer (not very effective), 
> or do you think I should contribute a patch to make the API public?
> 
> There's a third way, `getTrailingObjects()` is public in TrailingObjects, it 
> is possible to cast an `ParsedAttr` to a `TrailingObjects`, but I think it's 
> too hacky.
I take times trying and found out it's more difficult to use the original 
buffer, so I use a vector to collect the arguments instead.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92006

___
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

[Liao Chunyu via Phabricator via cfe-commits]

liaolucy added inline comments.



Comment at: clang/include/clang/Basic/RISCVVTypes.def:67
+RVV_VECTOR_TYPE_INT("__rvv_int8m2_t",  RvvInt8m2,  RvvInt8m2Ty,  16,  8, 1, 
true)
+RVV_VECTOR_TYPE_INT("__rvv_int8m4_t",  RvvInt8m4,  RvvInt8m4Ty,  32,  8, 1, 
true)
+RVV_VECTOR_TYPE_INT("__rvv_int8m8_t",  RvvInt8m8,  RvvInt8m8Ty,  64,  8, 1, 
true)

RISC-V V has too many types, more than 200. All types use builtin types? Is it 
possible to reduce the number of builtin types?


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] D92751: Precondition isHomogeneousAggregate on isCXX14Aggregate

[David Truby via Phabricator via cfe-commits]

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

MSVC on WoA64 includes isCXX14Aggregate in its definition. This is de-facto
specification on that platform, so match msvc's behaviour.

Fixes: https://bugs.llvm.org/show_bug.cgi?id=47611

Co-authored-by: Peter Waller 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D92751

Files:
  clang/lib/CodeGen/CGCXXABI.cpp
  clang/lib/CodeGen/CGCXXABI.h
  clang/lib/CodeGen/MicrosoftCXXABI.cpp
  clang/lib/CodeGen/TargetInfo.cpp
  clang/test/CodeGenCXX/microsoft-abi-sret-and-byval.cpp

Index: clang/test/CodeGenCXX/microsoft-abi-sret-and-byval.cpp
===
--- clang/test/CodeGenCXX/microsoft-abi-sret-and-byval.cpp
+++ clang/test/CodeGenCXX/microsoft-abi-sret-and-byval.cpp
@@ -468,3 +468,31 @@
 // WIN64-LABEL: define dso_local void @"?g@C@pr30293@@QEAAXXZ"(%"struct.pr30293::C"* {{[^,]*}} %this)
 // WIN64: declare dso_local void @"?h@C@pr30293@@UEAAXUSmallWithDtor@@@Z"(i8*, i32)
 }
+
+namespace pr47611 {
+// MSVC on Arm includes "isCXX14Aggregate" as part of its definition of
+// Homogeneous Floating-point Aggregate (HFA). Additionally, it has a different
+// handling of C++14 aggregates, which can lead to confusion.
+
+// Pod is a trivial HFA.
+struct Pod {
+  double b[2];
+};
+// Not an aggregate according to C++14 spec => not HFA according to MSVC.
+struct NotCXX14Aggregate {
+  NotCXX14Aggregate();
+  Pod p;
+};
+// NotPod is a C++14 aggregate. But not HFA, because it contains
+// NotCXX14Aggregate (which itself is not HFA because it's not a C++14
+// aggregate).
+struct NotPod {
+  NotCXX14Aggregate x;
+};
+// WOA64-LABEL: define dso_local %"struct.pr47611::Pod" @"?copy@pr47611@@YA?AUPod@1@PEAU21@@Z"(%"struct.pr47611::Pod"* %x)
+Pod copy(Pod *x) { return *x; }  // MSVC: ldp d0,d1,[x0], Clang: ldp d0,d1,[x0]
+// WOA64-LABEL: define dso_local void @"?copy@pr47611@@YA?AUNotCXX14Aggregate@1@PEAU21@@Z"(%"struct.pr47611::NotCXX14Aggregate"* inreg noalias sret(%"struct.pr47611::NotCXX14Aggregate") align 8 %agg.result, %"struct.pr47611::NotCXX14Aggregate"* %x)
+NotCXX14Aggregate copy(NotCXX14Aggregate *x) { return *x; } // MSVC: stp x8,x9,[x0], Clang: str q0,[x0]
+// WOA64-LABEL: define dso_local [2 x i64] @"?copy@pr47611@@YA?AUNotPod@1@PEAU21@@Z"(%"struct.pr47611::NotPod"* %x)
+NotPod copy(NotPod *x) { return *x; }
+};
Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -5133,6 +5133,14 @@
 
 Members += FldMembers;
   }
+
+  const auto TT = CGT.getTarget().getTriple();
+  // MSVC Windows on Arm64 considers a type not HFA if it is not an
+  // aggregate according to the C++14 spec. This is not consistent with the
+  // AAPCS64, but is defacto spec on that platform.
+  if (TT.isAArch64() && TT.isWindowsMSVCEnvironment() &&
+  !CGCXXABI::isCXX14Aggregate(CXXRD))
+return false;
 }
 
 for (const auto *FD : RD->fields()) {
Index: clang/lib/CodeGen/MicrosoftCXXABI.cpp
===
--- clang/lib/CodeGen/MicrosoftCXXABI.cpp
+++ clang/lib/CodeGen/MicrosoftCXXABI.cpp
@@ -1070,30 +1070,6 @@
   return isDeletingDtor(GD);
 }
 
-static bool isCXX14Aggregate(const CXXRecordDecl *RD) {
-  // For AArch64, we use the C++14 definition of an aggregate, so we also
-  // check for:
-  //   No private or protected non static data members.
-  //   No base classes
-  //   No virtual functions
-  // Additionally, we need to ensure that there is a trivial copy assignment
-  // operator, a trivial destructor and no user-provided constructors.
-  if (RD->hasProtectedFields() || RD->hasPrivateFields())
-return false;
-  if (RD->getNumBases() > 0)
-return false;
-  if (RD->isPolymorphic())
-return false;
-  if (RD->hasNonTrivialCopyAssignment())
-return false;
-  for (const CXXConstructorDecl *Ctor : RD->ctors())
-if (Ctor->isUserProvided())
-  return false;
-  if (RD->hasNonTrivialDestructor())
-return false;
-  return true;
-}
-
 bool MicrosoftCXXABI::classifyReturnType(CGFunctionInfo &FI) const {
   const CXXRecordDecl *RD = FI.getReturnType()->getAsCXXRecordDecl();
   if (!RD)
Index: clang/lib/CodeGen/CGCXXABI.h
===
--- clang/lib/CodeGen/CGCXXABI.h
+++ clang/lib/CodeGen/CGCXXABI.h
@@ -625,6 +625,8 @@
   virtual std::pair
   LoadVTablePtr(CodeGenFunction &CGF, Address This,
 const CXXRecordDecl *RD) = 0;
+
+  static bool isCXX14Aggregate(const CXXRecordDecl *RD);
 };
 
 // Create an instance of a C++ ABI class:
Index: clang/lib/CodeGen/CGCXXABI.cpp
===
--- clang/lib/CodeGen/CGCX

[PATCH] D92704: [clangd] Publish config file errors over LSP

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

Looks like this breaks tests on windows: http://45.33.8.238/win/29315/step_9.txt


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92704

___
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

[Jessica Clarke via Phabricator via cfe-commits]

jrtc27 added inline comments.



Comment at: clang/include/clang/Basic/RISCVVTypes.def:67
+RVV_VECTOR_TYPE_INT("__rvv_int8m2_t",  RvvInt8m2,  RvvInt8m2Ty,  16,  8, 1, 
true)
+RVV_VECTOR_TYPE_INT("__rvv_int8m4_t",  RvvInt8m4,  RvvInt8m4Ty,  32,  8, 1, 
true)
+RVV_VECTOR_TYPE_INT("__rvv_int8m8_t",  RvvInt8m8,  RvvInt8m8Ty,  64,  8, 1, 
true)

liaolucy wrote:
> RISC-V V has too many types, more than 200. All types use builtin types? Is 
> it possible to reduce the number of builtin types?
Indeed this is madness, what's wrong with just using 
`__attribute__((vector_size(n)))` on the right type? We should not be 
encouraging people to write code with architecture-specific types... but if we 
_really_ need these because RISC-V GCC decided this is how RISC-V V is going to 
look them can we not just shove them all in a header as typedef's for the 
architecture-independent attributed types and push that complexity out of the 
compiler itself?


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] D92753: [clang-format] Add IndentPragma style to eliminate common clang-format off scenario

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay created this revision.
MyDeveloperDay added reviewers: curdeius, JakeMerdichAMD, krasimir.
MyDeveloperDay added projects: clang-format, clang.
MyDeveloperDay requested review of this revision.
Herald added a reviewer: jdoerfert.
Herald added a subscriber: sstefan1.

A quick search of github.com, shows one common scenario for excessive use of 
//clang-format off/on is the indentation of #pragma's, especially around the 
areas of loop optimization or OpenMP

This revision aims to help that by introducing an `IndentPragmas` style, the 
aim of which is to keep the pragma at the current level of scope

  for (int i = 0; i < 5; i++) {
  // clang-format off
  #pragma HLS UNROLL
  // clang-format on
  for (int j = 0; j < 5; j++) {
  // clang-format off
  #pragma HLS UNROLL
  // clang-format on
   

can become

  for (int i = 0; i < 5; i++) {
  #pragma HLS UNROLL
  for (int j = 0; j < 5; j++) {
  #pragma HLS UNROLL
  

This revision also support working alongside the `IndentPPDirective` of 
`BeforeHash` and `AfterHash` (see unit tests for examples)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D92753

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/ContinuationIndenter.cpp
  clang/lib/Format/Format.cpp
  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
@@ -67,12 +67,13 @@
 
   void _verifyFormat(const char *File, int Line, llvm::StringRef Expected,
  llvm::StringRef Code,
- const FormatStyle &Style = getLLVMStyle()) {
+ const FormatStyle &Style = getLLVMStyle(),
+ bool messUp = true) {
 ScopedTrace t(File, Line, ::testing::Message() << Code.str());
 EXPECT_EQ(Expected.str(), format(Expected, Style))
 << "Expected code is not stable";
 EXPECT_EQ(Expected.str(), format(Code, Style));
-if (Style.Language == FormatStyle::LK_Cpp) {
+if (Style.Language == FormatStyle::LK_Cpp && messUp) {
   // Objective-C++ is a superset of C++, so everything checked for C++
   // needs to be checked for Objective-C++ as well.
   FormatStyle ObjCStyle = Style;
@@ -82,8 +83,10 @@
   }
 
   void _verifyFormat(const char *File, int Line, llvm::StringRef Code,
- const FormatStyle &Style = getLLVMStyle()) {
-_verifyFormat(File, Line, Code, test::messUp(Code), Style);
+ const FormatStyle &Style = getLLVMStyle(),
+ bool messUp = true) {
+_verifyFormat(File, Line, Code, messUp ? test::messUp(Code) : Code, Style,
+  messUp);
   }
 
   void _verifyIncompleteFormat(const char *File, int Line, llvm::StringRef Code,
@@ -14116,6 +14119,7 @@
   CHECK_PARSE_BOOL(IndentCaseLabels);
   CHECK_PARSE_BOOL(IndentCaseBlocks);
   CHECK_PARSE_BOOL(IndentGotoLabels);
+  CHECK_PARSE_BOOL(IndentPragmas);
   CHECK_PARSE_BOOL(IndentRequires);
   CHECK_PARSE_BOOL(IndentWrappedFunctionNames);
   CHECK_PARSE_BOOL(KeepEmptyLinesAtTheStartOfBlocks);
@@ -17562,6 +17566,119 @@
"struct constant;",
Style);
 }
+
+TEST_F(FormatTest, IndentPragmas) {
+  FormatStyle Style = getLLVMStyle();
+  Style.IndentPPDirectives = FormatStyle::PPDIS_None;
+
+  Style.IndentPragmas = false;
+  verifyFormat("#pragma once", Style);
+  verifyFormat("#pragma omp simd\n"
+   "for (int i = 0; i < 10; i++) {\n"
+   "}",
+   Style,
+   /*messUp=*/false);
+  verifyFormat("void foo() {\n"
+   "#pragma omp simd\n"
+   "  for (int i = 0; i < 10; i++) {\n"
+   "  }\n"
+   "}",
+   Style,
+   /*messUp=*/false);
+  verifyFormat("void foo() {\n"
+   "// outer loop\n"
+   "#pragma omp simd\n"
+   "  for (int k = 0; k < 10; k++) {\n"
+   "// inner loop\n"
+   "#pragma omp simd\n"
+   "for (int j = 0; j < 10; j++) {\n"
+   "}\n"
+   "  }\n"
+   "}",
+   Style,
+   /*messUp=*/false);
+
+  verifyFormat("void foo() {\n"
+   "// outer loop\n"
+   "#if 1\n"
+   "#pragma omp simd\n"
+   "  for (int k = 0; k < 10; k++) {\n"
+   "// inner loop\n"
+   "#pragma omp simd\n"
+   "for (int j = 0; j < 10; j++) {\n"
+   "}\n"
+   "  }\n"
+   "#endif\n"
+   "}",
+   Style,
+   /*messUp=*/false);
+

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

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

This needs rebasing against main. Can't be applied cleanly in its current state.




Comment at: 
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:398-404
+  std::string OptionVal = StrMap.lookup(OptionKey);
+  llvm::transform(OptionVal, OptionVal.begin(), toupper);
+
+  if (OptionVal == "1" || OptionVal == "TRUE" || OptionVal == "ON")
+return true;
+
+  return false;

There's no need to build a string and transform it here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86671

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

Re: [PATCH] D92704: [clangd] Publish config file errors over LSP

[Sam McCall via cfe-commits]

Yep, thanks. This uncovered an existing error, so rather than reverting the
whole thing I've disabled the assert on windows while I fix
it. f1357264b8e3070bef5bb4ff35ececa4d6c76108

On Mon, Dec 7, 2020 at 12:22 PM Nico Weber via Phabricator <
revi...@reviews.llvm.org> wrote:

> thakis added a comment.
>
> Looks like this breaks tests on windows:
> http://45.33.8.238/win/29315/step_9.txt
>
>
> Repository:
>   rG LLVM Github Monorepo
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D92704/new/
>
> https://reviews.llvm.org/D92704
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] f135726 - [clangd] Temporarily test that uncovered broken behavior on windows

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2020-12-07T12:34:17+01:00
New Revision: f1357264b8e3070bef5bb4ff35ececa4d6c76108

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

LOG: [clangd] Temporarily test that uncovered broken behavior on windows

Added: 


Modified: 
clang-tools-extra/clangd/unittests/ConfigProviderTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/unittests/ConfigProviderTests.cpp 
b/clang-tools-extra/clangd/unittests/ConfigProviderTests.cpp
index b2217bbc55da..4e44af6fad58 100644
--- a/clang-tools-extra/clangd/unittests/ConfigProviderTests.cpp
+++ b/clang-tools-extra/clangd/unittests/ConfigProviderTests.cpp
@@ -144,8 +144,11 @@ TEST(ProviderTest, FromAncestorRelativeYAMLFiles) {
   Cfg = P->getConfig(ABCParams, Diags.callback());
   EXPECT_THAT(Diags.Diagnostics,
   ElementsAre(DiagMessage("Unknown CompileFlags key Unknown")));
+#ifdef LLVM_ON_UNIX
+  // FIXME: fails on windows: paths have mixed slashes like C:\a/b\c.yaml
   EXPECT_THAT(Diags.Files,
   ElementsAre(testPath("a/foo.yaml"), testPath("a/b/c/foo.yaml")));
+#endif
   EXPECT_THAT(getAddedArgs(Cfg), ElementsAre("foo", "bar", "baz"));
   Diags.clear();
 



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

[PATCH] D92753: [clang-format] Add IndentPragma style to eliminate common clang-format off scenario

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

Should this be controlled on a case by case basis, maybe control indentation 
using a regex over the pragma arguments, WDYT?




Comment at: clang/docs/ClangFormatStyleOptions.rst:1923-1924
+
+  When ``false``, pragmas are flushed left or follow IndentPPDirectives
+  When ``true``, pragmas are indented to the current scope level
+

Please add full stops at the end of these sentences.



Comment at: clang/docs/ClangFormatStyleOptions.rst:1928
+
+false:   vs false:
+





Comment at: clang/include/clang/Format/Format.h:1533-1534
+  ///
+  /// When ``false``, pragmas are flushed left or follow IndentPPDirectives
+  /// When ``true``, pragmas are indented to the current scope level
+  /// \code

Please add full stops at the end of these sentences.



Comment at: clang/include/clang/Format/Format.h:1536
+  /// \code
+  ///   false:   vs false:
+  ///





Comment at: clang/lib/Format/ContinuationIndenter.cpp:594
+State.Line->First->startsSequence(tok::hash, tok::pp_pragma);
+// If indenting pragmas remove the extra space for the #
+if (Style.IndentPragmas && isPragmaLine) {

Full stop.



Comment at: clang/lib/Format/ContinuationIndenter.cpp:595-597
+if (Style.IndentPragmas && isPragmaLine) {
+  Spaces--;
+}

Can elide these braces.



Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:1248
+case FormatStyle::PPDIS_BeforeHash: {
+  // if we want to indent pragmas
+  bool isPragmaLine = RootToken.startsSequence(tok::hash, tok::pp_pragma);

Capital letter at the start of comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92753

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

[PATCH] D91950: [clang-format] Add BreakBeforeInlineASMColon configuration

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

I think my expectation would be that if you are going to have an option 
`BreakBeforeInlineASMColon` then it should do it for all case long and short, 
otherwise its not going to be clear.


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

https://reviews.llvm.org/D91950

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

[clang-tools-extra] 2542ef8 - [clangd] Fix windows slashes in project config diagnostics

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2020-12-07T12:54:38+01:00
New Revision: 2542ef83ed7c5a10f8b394ec8e7764558dc71d32

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

LOG: [clangd] Fix windows slashes in project config diagnostics

Added: 


Modified: 
clang-tools-extra/clangd/ConfigProvider.cpp
clang-tools-extra/clangd/unittests/ConfigProviderTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/ConfigProvider.cpp 
b/clang-tools-extra/clangd/ConfigProvider.cpp
index 8a3991ed1c1e..0933e7e2c283 100644
--- a/clang-tools-extra/clangd/ConfigProvider.cpp
+++ b/clang-tools-extra/clangd/ConfigProvider.cpp
@@ -83,7 +83,7 @@ Provider::fromAncestorRelativeYAMLFiles(llvm::StringRef 
RelPath,
 const ThreadsafeFS &FS;
 
 mutable std::mutex Mu;
-// Keys are the ancestor directory, not the actual config path within it.
+// Keys are the (posix-style) ancestor directory, not the config within it.
 // We only insert into this map, so pointers to values are stable forever.
 // Mutex guards the map itself, not the values (which are threadsafe).
 mutable llvm::StringMap Cache;
@@ -117,6 +117,8 @@ Provider::fromAncestorRelativeYAMLFiles(llvm::StringRef 
RelPath,
   if (It == Cache.end()) {
 llvm::SmallString<256> ConfigPath = Ancestor;
 path::append(ConfigPath, RelPath);
+// Use native slashes for reading the file, affects diagnostics.
+llvm::sys::path::native(ConfigPath);
 It = Cache.try_emplace(Ancestor, ConfigPath.str(), Ancestor).first;
   }
   Caches.push_back(&It->second);

diff  --git a/clang-tools-extra/clangd/unittests/ConfigProviderTests.cpp 
b/clang-tools-extra/clangd/unittests/ConfigProviderTests.cpp
index 4e44af6fad58..9c45266d1a83 100644
--- a/clang-tools-extra/clangd/unittests/ConfigProviderTests.cpp
+++ b/clang-tools-extra/clangd/unittests/ConfigProviderTests.cpp
@@ -144,11 +144,9 @@ TEST(ProviderTest, FromAncestorRelativeYAMLFiles) {
   Cfg = P->getConfig(ABCParams, Diags.callback());
   EXPECT_THAT(Diags.Diagnostics,
   ElementsAre(DiagMessage("Unknown CompileFlags key Unknown")));
-#ifdef LLVM_ON_UNIX
   // FIXME: fails on windows: paths have mixed slashes like C:\a/b\c.yaml
   EXPECT_THAT(Diags.Files,
   ElementsAre(testPath("a/foo.yaml"), testPath("a/b/c/foo.yaml")));
-#endif
   EXPECT_THAT(getAddedArgs(Cfg), ElementsAre("foo", "bar", "baz"));
   Diags.clear();
 



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

[PATCH] D92640: Add ability to load a FixedCompilationDatabase from a buffer.

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGa1cb9cbf5c49: Add ability to load a FixedCompilationDatabase 
from a buffer. (authored by sammccall).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92640

Files:
  clang/include/clang/Tooling/CompilationDatabase.h
  clang/lib/Tooling/CompilationDatabase.cpp
  clang/unittests/Tooling/CompilationDatabaseTest.cpp


Index: clang/unittests/Tooling/CompilationDatabaseTest.cpp
===
--- clang/unittests/Tooling/CompilationDatabaseTest.cpp
+++ clang/unittests/Tooling/CompilationDatabaseTest.cpp
@@ -543,6 +543,27 @@
   EXPECT_EQ(0ul, Database.getAllCompileCommands().size());
 }
 
+TEST(FixedCompilationDatabase, FromBuffer) {
+  const char *Data = R"(
+
+ -DFOO=BAR
+
+--baz
+
+  )";
+  std::string ErrorMsg;
+  auto CDB =
+  FixedCompilationDatabase::loadFromBuffer("/cdb/dir", Data, ErrorMsg);
+
+  std::vector Result = CDB->getCompileCommands("/foo/bar.cc");
+  ASSERT_EQ(1ul, Result.size());
+  EXPECT_EQ("/cdb/dir", Result.front().Directory);
+  EXPECT_EQ("/foo/bar.cc", Result.front().Filename);
+  EXPECT_THAT(
+  Result.front().CommandLine,
+  ElementsAre(EndsWith("clang-tool"), "-DFOO=BAR", "--baz", 
"/foo/bar.cc"));
+}
+
 TEST(ParseFixedCompilationDatabase, ReturnsNullOnEmptyArgumentList) {
   int Argc = 0;
   std::string ErrorMsg;
Index: clang/lib/Tooling/CompilationDatabase.cpp
===
--- clang/lib/Tooling/CompilationDatabase.cpp
+++ clang/lib/Tooling/CompilationDatabase.cpp
@@ -348,16 +348,24 @@
 ErrorMsg = "Error while opening fixed database: " + Result.message();
 return nullptr;
   }
+  return loadFromBuffer(llvm::sys::path::parent_path(Path),
+(*File)->getBuffer(), ErrorMsg);
+}
+
+std::unique_ptr
+FixedCompilationDatabase::loadFromBuffer(StringRef Directory, StringRef Data,
+ std::string &ErrorMsg) {
+  ErrorMsg.clear();
   std::vector Args;
-  for (llvm::StringRef Line :
-   llvm::make_range(llvm::line_iterator(**File), llvm::line_iterator())) {
+  StringRef Line;
+  while (!Data.empty()) {
+std::tie(Line, Data) = Data.split('\n');
 // Stray whitespace is almost certainly unintended.
 Line = Line.trim();
 if (!Line.empty())
   Args.push_back(Line.str());
   }
-  return std::make_unique(
-  llvm::sys::path::parent_path(Path), std::move(Args));
+  return std::make_unique(Directory, 
std::move(Args));
 }
 
 FixedCompilationDatabase::
Index: clang/include/clang/Tooling/CompilationDatabase.h
===
--- clang/include/clang/Tooling/CompilationDatabase.h
+++ clang/include/clang/Tooling/CompilationDatabase.h
@@ -184,11 +184,16 @@
   int &Argc, const char *const *Argv, std::string &ErrorMsg,
   Twine Directory = ".");
 
-  /// Reads flags from the given file, one-per line.
+  /// Reads flags from the given file, one-per-line.
   /// Returns nullptr and sets ErrorMessage if we can't read the file.
   static std::unique_ptr
   loadFromFile(StringRef Path, std::string &ErrorMsg);
 
+  /// Reads flags from the given buffer, one-per-line.
+  /// Directory is the command CWD, typically the parent of compile_flags.txt.
+  static std::unique_ptr
+  loadFromBuffer(StringRef Directory, StringRef Data, std::string &ErrorMsg);
+
   /// Constructs a compilation data base from a specified directory
   /// and command line.
   FixedCompilationDatabase(Twine Directory, ArrayRef CommandLine);


Index: clang/unittests/Tooling/CompilationDatabaseTest.cpp
===
--- clang/unittests/Tooling/CompilationDatabaseTest.cpp
+++ clang/unittests/Tooling/CompilationDatabaseTest.cpp
@@ -543,6 +543,27 @@
   EXPECT_EQ(0ul, Database.getAllCompileCommands().size());
 }
 
+TEST(FixedCompilationDatabase, FromBuffer) {
+  const char *Data = R"(
+
+ -DFOO=BAR
+
+--baz
+
+  )";
+  std::string ErrorMsg;
+  auto CDB =
+  FixedCompilationDatabase::loadFromBuffer("/cdb/dir", Data, ErrorMsg);
+
+  std::vector Result = CDB->getCompileCommands("/foo/bar.cc");
+  ASSERT_EQ(1ul, Result.size());
+  EXPECT_EQ("/cdb/dir", Result.front().Directory);
+  EXPECT_EQ("/foo/bar.cc", Result.front().Filename);
+  EXPECT_THAT(
+  Result.front().CommandLine,
+  ElementsAre(EndsWith("clang-tool"), "-DFOO=BAR", "--baz", "/foo/bar.cc"));
+}
+
 TEST(ParseFixedCompilationDatabase, ReturnsNullOnEmptyArgumentList) {
   int Argc = 0;
   std::string ErrorMsg;
Index: clang/lib/Tooling/CompilationDatabase.cpp
===
--- clang/lib/Tooling/CompilationDatabase.cpp
+++ clang/lib/Tooling/CompilationDatabase.cpp
@@ -348,16 +348,24 @@
 ErrorMsg = "Error while opening fixed database: " +

[PATCH] D92753: [clang-format] Add IndentPragma style to eliminate common clang-format off scenario

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay updated this revision to Diff 309869.
MyDeveloperDay added a comment.

Address review comments


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

https://reviews.llvm.org/D92753

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/ContinuationIndenter.cpp
  clang/lib/Format/Format.cpp
  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
@@ -67,12 +67,13 @@
 
   void _verifyFormat(const char *File, int Line, llvm::StringRef Expected,
  llvm::StringRef Code,
- const FormatStyle &Style = getLLVMStyle()) {
+ const FormatStyle &Style = getLLVMStyle(),
+ bool messUp = true) {
 ScopedTrace t(File, Line, ::testing::Message() << Code.str());
 EXPECT_EQ(Expected.str(), format(Expected, Style))
 << "Expected code is not stable";
 EXPECT_EQ(Expected.str(), format(Code, Style));
-if (Style.Language == FormatStyle::LK_Cpp) {
+if (Style.Language == FormatStyle::LK_Cpp && messUp) {
   // Objective-C++ is a superset of C++, so everything checked for C++
   // needs to be checked for Objective-C++ as well.
   FormatStyle ObjCStyle = Style;
@@ -82,8 +83,10 @@
   }
 
   void _verifyFormat(const char *File, int Line, llvm::StringRef Code,
- const FormatStyle &Style = getLLVMStyle()) {
-_verifyFormat(File, Line, Code, test::messUp(Code), Style);
+ const FormatStyle &Style = getLLVMStyle(),
+ bool messUp = true) {
+_verifyFormat(File, Line, Code, messUp ? test::messUp(Code) : Code, Style,
+  messUp);
   }
 
   void _verifyIncompleteFormat(const char *File, int Line, llvm::StringRef Code,
@@ -14116,6 +14119,7 @@
   CHECK_PARSE_BOOL(IndentCaseLabels);
   CHECK_PARSE_BOOL(IndentCaseBlocks);
   CHECK_PARSE_BOOL(IndentGotoLabels);
+  CHECK_PARSE_BOOL(IndentPragmas);
   CHECK_PARSE_BOOL(IndentRequires);
   CHECK_PARSE_BOOL(IndentWrappedFunctionNames);
   CHECK_PARSE_BOOL(KeepEmptyLinesAtTheStartOfBlocks);
@@ -17562,6 +17566,119 @@
"struct constant;",
Style);
 }
+
+TEST_F(FormatTest, IndentPragmas) {
+  FormatStyle Style = getLLVMStyle();
+  Style.IndentPPDirectives = FormatStyle::PPDIS_None;
+
+  Style.IndentPragmas = false;
+  verifyFormat("#pragma once", Style);
+  verifyFormat("#pragma omp simd\n"
+   "for (int i = 0; i < 10; i++) {\n"
+   "}",
+   Style,
+   /*messUp=*/false);
+  verifyFormat("void foo() {\n"
+   "#pragma omp simd\n"
+   "  for (int i = 0; i < 10; i++) {\n"
+   "  }\n"
+   "}",
+   Style,
+   /*messUp=*/false);
+  verifyFormat("void foo() {\n"
+   "// outer loop\n"
+   "#pragma omp simd\n"
+   "  for (int k = 0; k < 10; k++) {\n"
+   "// inner loop\n"
+   "#pragma omp simd\n"
+   "for (int j = 0; j < 10; j++) {\n"
+   "}\n"
+   "  }\n"
+   "}",
+   Style,
+   /*messUp=*/false);
+
+  verifyFormat("void foo() {\n"
+   "// outer loop\n"
+   "#if 1\n"
+   "#pragma omp simd\n"
+   "  for (int k = 0; k < 10; k++) {\n"
+   "// inner loop\n"
+   "#pragma omp simd\n"
+   "for (int j = 0; j < 10; j++) {\n"
+   "}\n"
+   "  }\n"
+   "#endif\n"
+   "}",
+   Style,
+   /*messUp=*/false);
+
+  Style.IndentPragmas = true;
+  verifyFormat("#pragma once", Style);
+  verifyFormat("#pragma omp simd\n"
+   "for (int i = 0; i < 10; i++) {\n"
+   "}",
+   Style,
+   /*messUp=*/false);
+  verifyFormat("void foo() {\n"
+   "  #pragma omp simd\n"
+   "  for (int i = 0; i < 10; i++) {\n"
+   "  }\n"
+   "}",
+   Style,
+   /*messUp=*/false);
+  verifyFormat("void foo() {\n"
+   "  #pragma omp simd\n"
+   "  for (int i = 0; i < 10; i++) {\n"
+   "#pragma omp simd\n"
+   "for (int j = 0; j < 10; j++) {\n"
+   "}\n"
+   "  }\n"
+   "}",
+   Style,
+   /*messUp=*/false);
+
+  Style.IndentPPDirectives = FormatStyle::PPDIS_AfterHash;
+
+  verifyFormat("void foo() {\n"
+   "# pragma omp simd\n"
+   "  for (int i = 0; i < 10; i++) {\n"
+

[clang] a1cb9cb - Add ability to load a FixedCompilationDatabase from a buffer.

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2020-12-07T13:07:10+01:00
New Revision: a1cb9cbf5c4939e78a6c3b3677cf8e3dbdf51932

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

LOG: Add ability to load a FixedCompilationDatabase from a buffer.

Previously, loading one from a file meant allowing the library to do the IO.
Clangd would prefer to do such IO itself (e.g. to allow caching).

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

Added: 


Modified: 
clang/include/clang/Tooling/CompilationDatabase.h
clang/lib/Tooling/CompilationDatabase.cpp
clang/unittests/Tooling/CompilationDatabaseTest.cpp

Removed: 




diff  --git a/clang/include/clang/Tooling/CompilationDatabase.h 
b/clang/include/clang/Tooling/CompilationDatabase.h
index b28a8a6d6e51..cbd57e9609aa 100644
--- a/clang/include/clang/Tooling/CompilationDatabase.h
+++ b/clang/include/clang/Tooling/CompilationDatabase.h
@@ -184,11 +184,16 @@ class FixedCompilationDatabase : public 
CompilationDatabase {
   int &Argc, const char *const *Argv, std::string &ErrorMsg,
   Twine Directory = ".");
 
-  /// Reads flags from the given file, one-per line.
+  /// Reads flags from the given file, one-per-line.
   /// Returns nullptr and sets ErrorMessage if we can't read the file.
   static std::unique_ptr
   loadFromFile(StringRef Path, std::string &ErrorMsg);
 
+  /// Reads flags from the given buffer, one-per-line.
+  /// Directory is the command CWD, typically the parent of compile_flags.txt.
+  static std::unique_ptr
+  loadFromBuffer(StringRef Directory, StringRef Data, std::string &ErrorMsg);
+
   /// Constructs a compilation data base from a specified directory
   /// and command line.
   FixedCompilationDatabase(Twine Directory, ArrayRef CommandLine);

diff  --git a/clang/lib/Tooling/CompilationDatabase.cpp 
b/clang/lib/Tooling/CompilationDatabase.cpp
index 79bb8c0ce09a..d339fd044c02 100644
--- a/clang/lib/Tooling/CompilationDatabase.cpp
+++ b/clang/lib/Tooling/CompilationDatabase.cpp
@@ -348,16 +348,24 @@ FixedCompilationDatabase::loadFromFile(StringRef Path, 
std::string &ErrorMsg) {
 ErrorMsg = "Error while opening fixed database: " + Result.message();
 return nullptr;
   }
+  return loadFromBuffer(llvm::sys::path::parent_path(Path),
+(*File)->getBuffer(), ErrorMsg);
+}
+
+std::unique_ptr
+FixedCompilationDatabase::loadFromBuffer(StringRef Directory, StringRef Data,
+ std::string &ErrorMsg) {
+  ErrorMsg.clear();
   std::vector Args;
-  for (llvm::StringRef Line :
-   llvm::make_range(llvm::line_iterator(**File), llvm::line_iterator())) {
+  StringRef Line;
+  while (!Data.empty()) {
+std::tie(Line, Data) = Data.split('\n');
 // Stray whitespace is almost certainly unintended.
 Line = Line.trim();
 if (!Line.empty())
   Args.push_back(Line.str());
   }
-  return std::make_unique(
-  llvm::sys::path::parent_path(Path), std::move(Args));
+  return std::make_unique(Directory, 
std::move(Args));
 }
 
 FixedCompilationDatabase::

diff  --git a/clang/unittests/Tooling/CompilationDatabaseTest.cpp 
b/clang/unittests/Tooling/CompilationDatabaseTest.cpp
index 168a1d6f0fb0..a3ea899e572e 100644
--- a/clang/unittests/Tooling/CompilationDatabaseTest.cpp
+++ b/clang/unittests/Tooling/CompilationDatabaseTest.cpp
@@ -543,6 +543,27 @@ TEST(FixedCompilationDatabase, GetAllCompileCommands) {
   EXPECT_EQ(0ul, Database.getAllCompileCommands().size());
 }
 
+TEST(FixedCompilationDatabase, FromBuffer) {
+  const char *Data = R"(
+
+ -DFOO=BAR
+
+--baz
+
+  )";
+  std::string ErrorMsg;
+  auto CDB =
+  FixedCompilationDatabase::loadFromBuffer("/cdb/dir", Data, ErrorMsg);
+
+  std::vector Result = CDB->getCompileCommands("/foo/bar.cc");
+  ASSERT_EQ(1ul, Result.size());
+  EXPECT_EQ("/cdb/dir", Result.front().Directory);
+  EXPECT_EQ("/foo/bar.cc", Result.front().Filename);
+  EXPECT_THAT(
+  Result.front().CommandLine,
+  ElementsAre(EndsWith("clang-tool"), "-DFOO=BAR", "--baz", 
"/foo/bar.cc"));
+}
+
 TEST(ParseFixedCompilationDatabase, ReturnsNullOnEmptyArgumentList) {
   int Argc = 0;
   std::string ErrorMsg;



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

[PATCH] D92652: [clang-tidy][docs] Update check options with boolean values instead of non-zero/0/1

[Nathan James via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG980618145bf0: [clang-tidy][docs] Update check options with 
boolean values instead of non… (authored by njames93).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92652

Files:
  clang-tools-extra/docs/clang-tidy/checks/bugprone-argument-comment.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone-misplaced-widening-cast.rst
  
clang-tools-extra/docs/clang-tidy/checks/bugprone-not-null-terminated-result.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone-reserved-identifier.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone-signed-char-misuse.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone-sizeof-expression.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone-string-constructor.rst
  
clang-tools-extra/docs/clang-tidy/checks/bugprone-suspicious-string-compare.rst
  
clang-tools-extra/docs/clang-tidy/checks/bugprone-unhandled-self-assignment.rst
  clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-macro-usage.rst
  
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-narrowing-conversions.rst
  
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-prefer-member-initializer.rst
  
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-pro-type-member-init.rst
  
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-special-member-functions.rst
  clang-tools-extra/docs/clang-tidy/checks/hicpp-multiway-paths-covered.rst
  clang-tools-extra/docs/clang-tidy/checks/misc-definitions-in-headers.rst
  
clang-tools-extra/docs/clang-tidy/checks/misc-throw-by-value-catch-by-reference.rst
  clang-tools-extra/docs/clang-tidy/checks/misc-unused-parameters.rst
  clang-tools-extra/docs/clang-tidy/checks/modernize-avoid-bind.rst
  clang-tools-extra/docs/clang-tidy/checks/modernize-make-shared.rst
  clang-tools-extra/docs/clang-tidy/checks/modernize-make-unique.rst
  clang-tools-extra/docs/clang-tidy/checks/modernize-pass-by-value.rst
  clang-tools-extra/docs/clang-tidy/checks/modernize-use-auto.rst
  clang-tools-extra/docs/clang-tidy/checks/modernize-use-bool-literals.rst
  clang-tools-extra/docs/clang-tidy/checks/modernize-use-default-member-init.rst
  clang-tools-extra/docs/clang-tidy/checks/modernize-use-emplace.rst
  clang-tools-extra/docs/clang-tidy/checks/modernize-use-equals-default.rst
  clang-tools-extra/docs/clang-tidy/checks/modernize-use-equals-delete.rst
  clang-tools-extra/docs/clang-tidy/checks/modernize-use-noexcept.rst
  clang-tools-extra/docs/clang-tidy/checks/modernize-use-override.rst
  
clang-tools-extra/docs/clang-tidy/checks/modernize-use-transparent-functors.rst
  clang-tools-extra/docs/clang-tidy/checks/modernize-use-using.rst
  clang-tools-extra/docs/clang-tidy/checks/performance-for-range-copy.rst
  
clang-tools-extra/docs/clang-tidy/checks/performance-inefficient-string-concatenation.rst
  
clang-tools-extra/docs/clang-tidy/checks/performance-inefficient-vector-operation.rst
  clang-tools-extra/docs/clang-tidy/checks/performance-move-const-arg.rst
  clang-tools-extra/docs/clang-tidy/checks/portability-simd-intrinsics.rst
  clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst
  
clang-tools-extra/docs/clang-tidy/checks/readability-implicit-bool-conversion.rst
  
clang-tools-extra/docs/clang-tidy/checks/readability-inconsistent-declaration-parameter-name.rst
  clang-tools-extra/docs/clang-tidy/checks/readability-qualified-auto.rst
  
clang-tools-extra/docs/clang-tidy/checks/readability-redundant-access-specifiers.rst
  clang-tools-extra/docs/clang-tidy/checks/readability-redundant-declaration.rst
  clang-tools-extra/docs/clang-tidy/checks/readability-redundant-member-init.rst
  
clang-tools-extra/docs/clang-tidy/checks/readability-redundant-smartptr-get.rst
  clang-tools-extra/docs/clang-tidy/checks/readability-simplify-boolean-expr.rst
  
clang-tools-extra/docs/clang-tidy/checks/readability-uppercase-literal-suffix.rst

Index: clang-tools-extra/docs/clang-tidy/checks/readability-uppercase-literal-suffix.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/readability-uppercase-literal-suffix.rst
+++ clang-tools-extra/docs/clang-tidy/checks/readability-uppercase-literal-suffix.rst
@@ -52,5 +52,5 @@
 
 .. option:: IgnoreMacros
 
-   If this option is set to non-zero (default is `1`), the check will not warn
+   If this option is set to `true` (default is `true`), the check will not warn
about literal suffixes inside macros.
Index: clang-tools-extra/docs/clang-tidy/checks/readability-simplify-boolean-expr.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/readability-simplify-boolean-expr.rst
+++ clang-tools-extra/docs/clang-tidy/checks/readability-simplify-boolean-expr.rst
@@ -77,10 +77,10 @@
 
 .. option:: ChainedConditi

[PATCH] D92753: [clang-format] Add IndentPragma style to eliminate common clang-format off scenario

[MyDeveloperDay via Phabricator via cfe-commits]

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

> Should this be controlled on a case by case basis, maybe control indentation 
> using a regex over the pragma arguments, WDYT?

Most uses of pragmas seem to be at the 0'th level of scrope `#pragma once` etc

Most other pragmas are probably with the code (like comments) as such it seems 
we should add an option to keep it with the scope of the code, I think a regex 
is overkill, but we could add it later if deemed necessary.


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

https://reviews.llvm.org/D92753

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

[clang-tools-extra] 9806181 - [clang-tidy][docs] Update check options with boolean values instead of non-zero/0/1

[Nathan James via cfe-commits]

Author: Nathan James
Date: 2020-12-07T12:13:57Z
New Revision: 980618145bf00a8e212cf3c6db46fb0a83081d69

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

LOG: [clang-tidy][docs] Update check options with boolean values instead of 
non-zero/0/1

Using bools instead of integers better conveys the expected value of the option.

Reviewed By: Eugene.Zelenko, aaron.ballman

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

Added: 


Modified: 
clang-tools-extra/docs/clang-tidy/checks/bugprone-argument-comment.rst

clang-tools-extra/docs/clang-tidy/checks/bugprone-misplaced-widening-cast.rst

clang-tools-extra/docs/clang-tidy/checks/bugprone-not-null-terminated-result.rst
clang-tools-extra/docs/clang-tidy/checks/bugprone-reserved-identifier.rst
clang-tools-extra/docs/clang-tidy/checks/bugprone-signed-char-misuse.rst
clang-tools-extra/docs/clang-tidy/checks/bugprone-sizeof-expression.rst
clang-tools-extra/docs/clang-tidy/checks/bugprone-string-constructor.rst

clang-tools-extra/docs/clang-tidy/checks/bugprone-suspicious-string-compare.rst

clang-tools-extra/docs/clang-tidy/checks/bugprone-unhandled-self-assignment.rst
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-macro-usage.rst

clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-narrowing-conversions.rst

clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-prefer-member-initializer.rst

clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-pro-type-member-init.rst

clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-special-member-functions.rst
clang-tools-extra/docs/clang-tidy/checks/hicpp-multiway-paths-covered.rst
clang-tools-extra/docs/clang-tidy/checks/misc-definitions-in-headers.rst

clang-tools-extra/docs/clang-tidy/checks/misc-throw-by-value-catch-by-reference.rst
clang-tools-extra/docs/clang-tidy/checks/misc-unused-parameters.rst
clang-tools-extra/docs/clang-tidy/checks/modernize-avoid-bind.rst
clang-tools-extra/docs/clang-tidy/checks/modernize-make-shared.rst
clang-tools-extra/docs/clang-tidy/checks/modernize-make-unique.rst
clang-tools-extra/docs/clang-tidy/checks/modernize-pass-by-value.rst
clang-tools-extra/docs/clang-tidy/checks/modernize-use-auto.rst
clang-tools-extra/docs/clang-tidy/checks/modernize-use-bool-literals.rst

clang-tools-extra/docs/clang-tidy/checks/modernize-use-default-member-init.rst
clang-tools-extra/docs/clang-tidy/checks/modernize-use-emplace.rst
clang-tools-extra/docs/clang-tidy/checks/modernize-use-equals-default.rst
clang-tools-extra/docs/clang-tidy/checks/modernize-use-equals-delete.rst
clang-tools-extra/docs/clang-tidy/checks/modernize-use-noexcept.rst
clang-tools-extra/docs/clang-tidy/checks/modernize-use-override.rst

clang-tools-extra/docs/clang-tidy/checks/modernize-use-transparent-functors.rst
clang-tools-extra/docs/clang-tidy/checks/modernize-use-using.rst
clang-tools-extra/docs/clang-tidy/checks/performance-for-range-copy.rst

clang-tools-extra/docs/clang-tidy/checks/performance-inefficient-string-concatenation.rst

clang-tools-extra/docs/clang-tidy/checks/performance-inefficient-vector-operation.rst
clang-tools-extra/docs/clang-tidy/checks/performance-move-const-arg.rst
clang-tools-extra/docs/clang-tidy/checks/portability-simd-intrinsics.rst
clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst

clang-tools-extra/docs/clang-tidy/checks/readability-implicit-bool-conversion.rst

clang-tools-extra/docs/clang-tidy/checks/readability-inconsistent-declaration-parameter-name.rst
clang-tools-extra/docs/clang-tidy/checks/readability-qualified-auto.rst

clang-tools-extra/docs/clang-tidy/checks/readability-redundant-access-specifiers.rst

clang-tools-extra/docs/clang-tidy/checks/readability-redundant-declaration.rst

clang-tools-extra/docs/clang-tidy/checks/readability-redundant-member-init.rst

clang-tools-extra/docs/clang-tidy/checks/readability-redundant-smartptr-get.rst

clang-tools-extra/docs/clang-tidy/checks/readability-simplify-boolean-expr.rst

clang-tools-extra/docs/clang-tidy/checks/readability-uppercase-literal-suffix.rst

Removed: 




diff  --git 
a/clang-tools-extra/docs/clang-tidy/checks/bugprone-argument-comment.rst 
b/clang-tools-extra/docs/clang-tidy/checks/bugprone-argument-comment.rst
index 8c59541b8d42..ab7e668b971c 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/bugprone-argument-comment.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone-argument-comment.rst
@@ -24,17 +24,17 @@ Options
 
 .. option:: StrictMode
 
-   When zero (default value), the check will ignore leading and trailing
+   When `false` (default valu

[PATCH] D88220: [C++20] P1825R0: More implicit moves

[Yang Fan via Phabricator via cfe-commits]

nullptr.cpp updated this revision to Diff 309873.
nullptr.cpp added a comment.
Herald added a subscriber: lxfind.

Update


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88220

Files:
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaCoroutine.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/test/CXX/class/class.init/class.copy.elision/p3.cpp
  clang/test/SemaCXX/P1155.cpp
  clang/test/SemaCXX/warn-return-std-move.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -1203,6 +1203,11 @@
   https://wg21.link/p0593r6";>P0593R6 (DR)
   Clang 11
 
+
+More implicit moves
+https://wg21.link/p1825r0";>P1825R0 (DR)
+Clang 12
+
 
 
 
Index: clang/test/SemaCXX/warn-return-std-move.cpp
===
--- clang/test/SemaCXX/warn-return-std-move.cpp
+++ clang/test/SemaCXX/warn-return-std-move.cpp
@@ -1,5 +1,11 @@
-// RUN: %clang_cc1 -fsyntax-only -fcxx-exceptions -Wreturn-std-move -Wreturn-std-move-in-c++11 -std=c++14 -verify %s
-// RUN: %clang_cc1 -fsyntax-only -fcxx-exceptions -Wreturn-std-move -Wreturn-std-move-in-c++11 -std=c++14 -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -fsyntax-only -fcxx-exceptions -Wreturn-std-move -std=c++20 -verify=cxx20 %s
+// RUN: %clang_cc1 -fsyntax-only -fcxx-exceptions -Wreturn-std-move -std=c++17 -verify=cxx11_14_17 %s
+// RUN: %clang_cc1 -fsyntax-only -fcxx-exceptions -Wreturn-std-move -std=c++14 -verify=cxx11_14_17 %s
+// RUN: %clang_cc1 -fsyntax-only -fcxx-exceptions -Wreturn-std-move -std=c++11 -verify=cxx11_14_17 %s
+// RUN: %clang_cc1 -fsyntax-only -fcxx-exceptions -Wreturn-std-move -std=c++17 -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s -check-prefix=CXX11_14_17-CHECK
+// RUN: %clang_cc1 -fsyntax-only -fcxx-exceptions -Wreturn-std-move -std=c++14 -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s -check-prefix=CXX11_14_17-CHECK
+// RUN: %clang_cc1 -fsyntax-only -fcxx-exceptions -Wreturn-std-move -std=c++11 -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s -check-prefix=CXX11_14_17-CHECK
+// cxx20-no-diagnostics
 
 // definitions for std::move
 namespace std {
@@ -71,37 +77,34 @@
 Base test2() {
 Derived d2;
 return d2;  // e1
-// expected-warning@-1{{will be copied despite being returned by name}}
-// expected-note@-2{{to avoid copying}}
-// CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:12-[[@LINE-3]]:14}:"std::move(d2)"
+// cxx11_14_17-warning@-1{{will be copied despite being returned by name}}
+// cxx11_14_17-note@-2{{to avoid copying}}
+// CXX11_14_17-CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:12-[[@LINE-3]]:14}:"std::move(d2)"
 }
 ConstructFromDerived test3() {
 Derived d3;
-return d3;  // e2-cxx11
-// expected-warning@-1{{would have been copied despite being returned by name}}
-// expected-note@-2{{to avoid copying on older compilers}}
-// CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:12-[[@LINE-3]]:14}:"std::move(d3)"
+return d3;  // ok
 }
 ConstructFromBase test4() {
 Derived d4;
 return d4;  // e3
-// expected-warning@-1{{will be copied despite being returned by name}}
-// expected-note@-2{{to avoid copying}}
-// CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:12-[[@LINE-3]]:14}:"std::move(d4)"
+// cxx11_14_17-warning@-1{{will be copied despite being returned by name}}
+// cxx11_14_17-note@-2{{to avoid copying}}
+// CXX11_14_17-CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:12-[[@LINE-3]]:14}:"std::move(d4)"
 }
 ConvertFromDerived test5() {
 Derived d5;
 return d5;  // e4
-// expected-warning@-1{{will be copied despite being returned by name}}
-// expected-note@-2{{to avoid copying}}
-// CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:12-[[@LINE-3]]:14}:"std::move(d5)"
+// cxx11_14_17-warning@-1{{will be copied despite being returned by name}}
+// cxx11_14_17-note@-2{{to avoid copying}}
+// CXX11_14_17-CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:12-[[@LINE-3]]:14}:"std::move(d5)"
 }
 ConvertFromBase test6() {
 Derived d6;
 return d6;  // e5
-// expected-warning@-1{{will be copied despite being returned by name}}
-// expected-note@-2{{to avoid copying}}
-// CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:12-[[@LINE-3]]:14}:"std::move(d6)"
+// cxx11_14_17-warning@-1{{will be copied despite being returned by name}}
+// cxx11_14_17-note@-2{{to avoid copying}}
+// CXX11_14_17-CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:12-[[@LINE-3]]:14}:"std::move(d6)"
 }
 
 // These test cases should not produce the warning.
@@ -148,93 +151,89 @@
 Derived testParam1(Derived d) { return d; }
 Base testParam2(Derived d) {
 return d;  // e6
-// expected-warning@-1{

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

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

I tend to agree with @krasimir I don't see where you really use Maximum to mean 
anything, the nested configuration seems perhaps unnecessarily confusing?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92257

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

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

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added inline comments.



Comment at: clang/lib/Format/Format.cpp:893
  /*BeforeLambdaBody=*/false,
+ /*BeforeStructInitialization=*/false,
  /*BeforeWhile=*/false,

anastasiia_lukianenko wrote:
> MyDeveloperDay wrote:
> > I believe there are 3 places where BraceWrapping is initialized you seem to 
> > only be doing 2 of them
> Sorry, I didn't find one more place. Can you please say where it is?
`expandPresets()` - twice
`getLLVMStyle()` - once





Comment at: clang/unittests/Format/FormatTest.cpp:5063
+   "a = 1,\n"
+   "b = 2,\n"
+   "};",

anastasiia_lukianenko wrote:
> MyDeveloperDay wrote:
> > could you add an additional test without the trailing `,`
> Yes, I can add the test, but before I want to discuss the expected result.
> Now with my patch and BeforeStructInitialization = false the behavior is the 
> following:
> 
> ```
> struct new_struct struct_name = {a = 1};
> struct new_struct struct_name = {a = 1, b = 2};
> ```
> And with BeforeStructInitialization = true:
> 
> ```
> struct new_struct struct_name =
> {a = 1};
> struct new_struct struct_name =
> {a = 1, b = 2};
> ```
> Is it okay?
that is kind of what I expected, and adding the `,` is a known trick to cause 
it to expand the struct out.

I think its worth putting a test in, but I'll ask the same question what do you 
think it should look like?


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

https://reviews.llvm.org/D91949

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

[PATCH] D81678: Introduce noundef attribute at call sites for stricter poison analysis

[Juneyoung Lee via Phabricator via cfe-commits]

aqjune added a comment.

Hello all,
What is the status of this patch?
Do we need someone who look into the lit change?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81678

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

[PATCH] D92753: [clang-format] Add IndentPragma style to eliminate common clang-format off scenario

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added a comment.

This LGTM in general. I just have a doubt.
You've added `messUp` parameter to `verifyFormat`, because, IIUC, pragmas 
wouldn't be at the desired scope level indentation otherwise.
Shouldn't clang-format find out what the correct indentation level for pragmas 
should be instead of keeping the current indentation (when IndentPragmas: true)?
Correct me if my understanding is wrong.
Anyway, my point is that turning off messUp seems like a bad idea and may hide 
some problems, nope?

What would be the output of:

  void foo() {
  #pragma omp
for (...) {
  #pragma omp
  for (...) {
  }
}
  }

Do the `#pragma`s get intended to the level of corresponding `for`s or not?


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

https://reviews.llvm.org/D92753

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

[PATCH] D88220: [C++20] P1825R0: More implicit moves

[Yang Fan via Phabricator via cfe-commits]

nullptr.cpp added inline comments.



Comment at: clang/include/clang/Sema/Sema.h:4626
 CES_AllowExceptionVariables = 4,
-CES_FormerDefault = (CES_AllowParameters),
-CES_Default = (CES_AllowParameters | CES_AllowDifferentTypes),
-CES_AsIfByStdMove = (CES_AllowParameters | CES_AllowDifferentTypes |
- CES_AllowExceptionVariables),
+CES_AllowRValueReferenceType = 8,
+CES_ImplicitlyMovableCXX11CXX14CXX17 =

Quuxplusone wrote:
> I believe `RValue` should be spelled `Rvalue`, throughout.
There are already names such as `RValueReferenceType`, so be consistent with 
existing code.



Comment at: clang/test/CXX/class/class.init/class.copy.elision/p3.cpp:28
+private:
+  C(C &&); // cxx20-note {{declared private here}}
+};

[over.match.funcs]p8:
> A defaulted move special member function that is defined as deleted is 
> excluded from the set of candidate functions in all contexts.

So in this case still use private.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88220

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

[PATCH] D92756: [clang-tidy] Support all YAML supported spellings for bools in CheckOptions.

[Nathan James via Phabricator via cfe-commits]

njames93 created this revision.
njames93 added a reviewer: aaron.ballman.
Herald added a subscriber: xazax.hun.
njames93 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Depends on D92755 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D92756

Files:
  clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
  clang-tools-extra/docs/ReleaseNotes.rst


Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -73,6 +73,9 @@
   ` and
   :doc:`modernize-make-unique `.
 
+- CheckOptions that take boolean values now support all spellings supported in 
+  the `YAML format `_.
+
 New modules
 ^^^
 
Index: clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
===
--- clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
+++ clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
@@ -11,6 +11,7 @@
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/WithColor.h"
+#include "llvm/Support/YAMLParser.h"
 #include "llvm/Support/raw_ostream.h"
 
 namespace clang {
@@ -101,13 +102,11 @@
 
 static llvm::Expected getAsBool(StringRef Value,
   const llvm::Twine &LookupName) {
-  if (Value == "true")
-return true;
-  if (Value == "false")
-return false;
-  bool Result;
-  if (!Value.getAsInteger(10, Result))
-return Result;
+  // To maintain backwards compatability, we support parsing numbers as
+  // booleans, even though its not supported in YAML.
+  if (llvm::Optional Parsed =
+  llvm::yaml::parseBool(Value, /*AllowNumeric=*/true))
+return *Parsed;
   return llvm::make_error(LookupName.str(),
  Value.str(), true);
 }


Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -73,6 +73,9 @@
   ` and
   :doc:`modernize-make-unique `.
 
+- CheckOptions that take boolean values now support all spellings supported in 
+  the `YAML format `_.
+
 New modules
 ^^^
 
Index: clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
===
--- clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
+++ clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
@@ -11,6 +11,7 @@
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/WithColor.h"
+#include "llvm/Support/YAMLParser.h"
 #include "llvm/Support/raw_ostream.h"
 
 namespace clang {
@@ -101,13 +102,11 @@
 
 static llvm::Expected getAsBool(StringRef Value,
   const llvm::Twine &LookupName) {
-  if (Value == "true")
-return true;
-  if (Value == "false")
-return false;
-  bool Result;
-  if (!Value.getAsInteger(10, Result))
-return Result;
+  // To maintain backwards compatability, we support parsing numbers as
+  // booleans, even though its not supported in YAML.
+  if (llvm::Optional Parsed =
+  llvm::yaml::parseBool(Value, /*AllowNumeric=*/true))
+return *Parsed;
   return llvm::make_error(LookupName.str(),
  Value.str(), true);
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D92721: [PATCH] [clang] Create SPIRABIInfo to enable SPIR_FUNC calling convention

[Melanie Blower via Phabricator via cfe-commits]

mibintc planned changes to this revision.
mibintc added a comment.

I'm going to make further changes to this patch, it's not working as desired.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92721

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

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

[Anastasiia Lukianenko via Phabricator via cfe-commits]

anastasiia_lukianenko added inline comments.



Comment at: clang/lib/Format/Format.cpp:893
  /*BeforeLambdaBody=*/false,
+ /*BeforeStructInitialization=*/false,
  /*BeforeWhile=*/false,

MyDeveloperDay wrote:
> anastasiia_lukianenko wrote:
> > MyDeveloperDay wrote:
> > > I believe there are 3 places where BraceWrapping is initialized you seem 
> > > to only be doing 2 of them
> > Sorry, I didn't find one more place. Can you please say where it is?
> `expandPresets()` - twice
> `getLLVMStyle()` - once
> 
> 
Ok, thank you.



Comment at: clang/unittests/Format/FormatTest.cpp:5063
+   "a = 1,\n"
+   "b = 2,\n"
+   "};",

MyDeveloperDay wrote:
> anastasiia_lukianenko wrote:
> > MyDeveloperDay wrote:
> > > could you add an additional test without the trailing `,`
> > Yes, I can add the test, but before I want to discuss the expected result.
> > Now with my patch and BeforeStructInitialization = false the behavior is 
> > the following:
> > 
> > ```
> > struct new_struct struct_name = {a = 1};
> > struct new_struct struct_name = {a = 1, b = 2};
> > ```
> > And with BeforeStructInitialization = true:
> > 
> > ```
> > struct new_struct struct_name =
> > {a = 1};
> > struct new_struct struct_name =
> > {a = 1, b = 2};
> > ```
> > Is it okay?
> that is kind of what I expected, and adding the `,` is a known trick to cause 
> it to expand the struct out.
> 
> I think its worth putting a test in, but I'll ask the same question what do 
> you think it should look like?
I agree with you. And I'll add these tests to patch.


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

https://reviews.llvm.org/D91949

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

[PATCH] D88220: [C++20] P1825R0: More implicit moves

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

Quuxplusone added inline comments.



Comment at: clang/include/clang/Sema/Sema.h:4626
 CES_AllowExceptionVariables = 4,
-CES_FormerDefault = (CES_AllowParameters),
-CES_Default = (CES_AllowParameters | CES_AllowDifferentTypes),
-CES_AsIfByStdMove = (CES_AllowParameters | CES_AllowDifferentTypes |
- CES_AllowExceptionVariables),
+CES_AllowRValueReferenceType = 8,
+CES_ImplicitlyMovableCXX11CXX14CXX17 =

nullptr.cpp wrote:
> Quuxplusone wrote:
> > I believe `RValue` should be spelled `Rvalue`, throughout.
> There are already names such as `RValueReferenceType`, so be consistent with 
> existing code.
Okay, I see examples both ways and wish there were one standard spelling, but I 
agree there's no standard at the moment, so whatever.



Comment at: clang/test/CXX/class/class.init/class.copy.elision/p3.cpp:28
+private:
+  C(C &&); // cxx20-note {{declared private here}}
+};

nullptr.cpp wrote:
> [over.match.funcs]p8:
> > A defaulted move special member function that is defined as deleted is 
> > excluded from the set of candidate functions in all contexts.
> 
> So in this case still use private.
If it's user-declared as deleted, then it is not defaulted.
To get a move-constructor that is defaulted as deleted, you'd have to write 
something like https://godbolt.org/z/KbG76v (Notice how the behavior changes if 
you explicitly delete it with `C(C&&) = delete;` instead of implicitly deleting 
it with `C(C&&) = default;`.)



Comment at: clang/test/CXX/class/class.init/class.copy.elision/p3.cpp:22
+  return c;
+}
+#else

davidstone wrote:
> Quuxplusone wrote:
> > Quuxplusone wrote:
> > > @rsmith @david_stone (or anyone), what is the status in C++20 of the 
> > > following test case?
> > > 
> > > C&& test(C&& c) {
> > > return c;
> > > }
> > > 
> > > I know we talked about this in person at CppCon 2018, and concluded that 
> > > our //intention// was for this to be legal, but that it wasn't actually 
> > > legal as-worded, because the returned thingie here is not an object but 
> > > rather a reference, and therefore none of 
> > > [P1825's](http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2019/p1825r0.html)
> > >  wording actually covers it. Is that still the case? Is there an open 
> > > issue about this? Is there any appetite for Clang to just go ahead and 
> > > //make// this legal?  (The current patch does //not// make this legal.)
> > > 
> > > Relevant reading: 
> > > https://quuxplusone.github.io/blog/2018/09/25/perfect-backwarding/
> > Apparently @davidstone has been active more recently than @david_stone... :)
> Going through the wording in http://eel.is/c++draft/class.copy.elision#3
> 
> "An implicitly movable entity is a variable of automatic storage duration 
> that is either a non-volatile object or an rvalue reference to a non-volatile 
> object type."
> 
> So we are fine with a reference as the source. However, it then goes on to say
> 
> "...overload resolution to select the constructor for the copy or the 
> return_­value overload to call is first performed as if the expression or 
> operand were an rvalue."
> 
> There is technically no constructor to call here, which I think means this 
> section does not apply.
> 
> I don't believe an issue has been raised for this yet, so I'll email CWG 
> about it.
I don't know if there's a CWG issue number (yet?) but at least I have drafted a 
paper, 
[D2266R0](http://quuxplusone.github.io/draft/d2266-implicit-move-rvalue-ref.html),
 on the subject. @nullptr.cpp could you take a look at this paper and let me 
know (perhaps by email) what you think about it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88220

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

[clang-tools-extra] 8625f5b - [clang-tidy][NFC] Streamline CheckOptions error reporting.

[Nathan James via cfe-commits]

Author: Nathan James
Date: 2020-12-07T14:05:49Z
New Revision: 8625f5bc799f4ee1c85126bd007166fe6dff14a1

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

LOG: [clang-tidy][NFC] Streamline CheckOptions error reporting.

Added: 


Modified: 
clang-tools-extra/clang-tidy/ClangTidyCheck.cpp

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/ClangTidyCheck.cpp 
b/clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
index c99931e0aa3a..be68dfbedb29 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
+++ b/clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
@@ -21,28 +21,22 @@ char UnparseableEnumOptionError::ID;
 char UnparseableIntegerOptionError::ID;
 
 std::string MissingOptionError::message() const {
-  llvm::SmallString<128> Buffer;
-  llvm::raw_svector_ostream Output(Buffer);
-  Output << "option not found '" << OptionName << '\'';
+  llvm::SmallString<128> Buffer({"option not found '", OptionName, "'"});
   return std::string(Buffer);
 }
 
 std::string UnparseableEnumOptionError::message() const {
-  llvm::SmallString<128> Buffer;
-  llvm::raw_svector_ostream Output(Buffer);
-  Output << "invalid configuration value '" << LookupValue << "' for option '"
- << LookupName << '\'';
+  llvm::SmallString<256> Buffer({"invalid configuration value '", LookupValue,
+ "' for option '", LookupName, "'"});
   if (SuggestedValue)
-Output << "; did you mean '" << *SuggestedValue << "'?";
+Buffer.append({"; did you mean '", *SuggestedValue, "'?"});
   return std::string(Buffer);
 }
 
 std::string UnparseableIntegerOptionError::message() const {
-  llvm::SmallString<128> Buffer;
-  llvm::raw_svector_ostream Output(Buffer);
-  Output << "invalid configuration value '" << LookupValue << "' for option '"
- << LookupName << "'; expected "
- << (IsBoolean ? "a bool" : "an integer value");
+  llvm::SmallString<256> Buffer({"invalid configuration value '", LookupValue,
+ "' for option '", LookupName, "'; expected ",
+ (IsBoolean ? "a bool" : "an integer value")});
   return std::string(Buffer);
 }
 



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

[PATCH] D92753: [clang-format] Add IndentPragma style to eliminate common clang-format off scenario

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

In D92753#2436852 , @curdeius wrote:

> You've added `messUp` parameter to `verifyFormat`, because, IIUC, pragmas 
> wouldn't be at the desired scope level indentation otherwise.

messUp was pull the line below onto the pragma line, which was then unable to 
determine when if it should break it

i.e.

  #pragma omp simd
  for (int k = 0; k < 10; k++) {

became

  #pragma omp simd for (int k = 0; k < 10; k++) {

I'm not sure why this occurred, but I assume there is no real token parsing of 
an #pragam line.


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

https://reviews.llvm.org/D92753

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

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

[Felix Berger via Phabricator via cfe-commits]

flx added a comment.

Could someone take a first pass at this change? It would be great progress on 
this as this is the last currently known case that generates false positives.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91893

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

[PATCH] D92753: [clang-format] Add IndentPragma style to eliminate common clang-format off scenario

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay updated this revision to Diff 309896.
MyDeveloperDay added a comment.

Add for(...) test case


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

https://reviews.llvm.org/D92753

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/ContinuationIndenter.cpp
  clang/lib/Format/Format.cpp
  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
@@ -67,12 +67,13 @@
 
   void _verifyFormat(const char *File, int Line, llvm::StringRef Expected,
  llvm::StringRef Code,
- const FormatStyle &Style = getLLVMStyle()) {
+ const FormatStyle &Style = getLLVMStyle(),
+ bool messUp = true) {
 ScopedTrace t(File, Line, ::testing::Message() << Code.str());
 EXPECT_EQ(Expected.str(), format(Expected, Style))
 << "Expected code is not stable";
 EXPECT_EQ(Expected.str(), format(Code, Style));
-if (Style.Language == FormatStyle::LK_Cpp) {
+if (Style.Language == FormatStyle::LK_Cpp && messUp) {
   // Objective-C++ is a superset of C++, so everything checked for C++
   // needs to be checked for Objective-C++ as well.
   FormatStyle ObjCStyle = Style;
@@ -82,8 +83,10 @@
   }
 
   void _verifyFormat(const char *File, int Line, llvm::StringRef Code,
- const FormatStyle &Style = getLLVMStyle()) {
-_verifyFormat(File, Line, Code, test::messUp(Code), Style);
+ const FormatStyle &Style = getLLVMStyle(),
+ bool messUp = true) {
+_verifyFormat(File, Line, Code, messUp ? test::messUp(Code) : Code, Style,
+  messUp);
   }
 
   void _verifyIncompleteFormat(const char *File, int Line, llvm::StringRef Code,
@@ -14116,6 +14119,7 @@
   CHECK_PARSE_BOOL(IndentCaseLabels);
   CHECK_PARSE_BOOL(IndentCaseBlocks);
   CHECK_PARSE_BOOL(IndentGotoLabels);
+  CHECK_PARSE_BOOL(IndentPragmas);
   CHECK_PARSE_BOOL(IndentRequires);
   CHECK_PARSE_BOOL(IndentWrappedFunctionNames);
   CHECK_PARSE_BOOL(KeepEmptyLinesAtTheStartOfBlocks);
@@ -17562,6 +17566,130 @@
"struct constant;",
Style);
 }
+
+TEST_F(FormatTest, IndentPragmas) {
+  FormatStyle Style = getLLVMStyle();
+  Style.IndentPPDirectives = FormatStyle::PPDIS_None;
+
+  Style.IndentPragmas = false;
+  verifyFormat("#pragma once", Style);
+  verifyFormat("#pragma omp simd\n"
+   "for (int i = 0; i < 10; i++) {\n"
+   "}",
+   Style,
+   /*messUp=*/false);
+  verifyFormat("void foo() {\n"
+   "#pragma omp simd\n"
+   "  for (int i = 0; i < 10; i++) {\n"
+   "  }\n"
+   "}",
+   Style,
+   /*messUp=*/false);
+  verifyFormat("void foo() {\n"
+   "// outer loop\n"
+   "#pragma omp simd\n"
+   "  for (int k = 0; k < 10; k++) {\n"
+   "// inner loop\n"
+   "#pragma omp simd\n"
+   "for (int j = 0; j < 10; j++) {\n"
+   "}\n"
+   "  }\n"
+   "}",
+   Style,
+   /*messUp=*/false);
+
+  verifyFormat("void foo() {\n"
+   "// outer loop\n"
+   "#if 1\n"
+   "#pragma omp simd\n"
+   "  for (int k = 0; k < 10; k++) {\n"
+   "// inner loop\n"
+   "#pragma omp simd\n"
+   "for (int j = 0; j < 10; j++) {\n"
+   "}\n"
+   "  }\n"
+   "#endif\n"
+   "}",
+   Style,
+   /*messUp=*/false);
+
+  Style.IndentPragmas = true;
+  verifyFormat("#pragma once", Style);
+  verifyFormat("#pragma omp simd\n"
+   "for (int i = 0; i < 10; i++) {\n"
+   "}",
+   Style,
+   /*messUp=*/false);
+  verifyFormat("void foo() {\n"
+   "  #pragma omp simd\n"
+   "  for (int i = 0; i < 10; i++) {\n"
+   "  }\n"
+   "}",
+   Style,
+   /*messUp=*/false);
+  verifyFormat("void foo() {\n"
+   "  #pragma omp simd\n"
+   "  for (int i = 0; i < 10; i++) {\n"
+   "#pragma omp simd\n"
+   "for (int j = 0; j < 10; j++) {\n"
+   "}\n"
+   "  }\n"
+   "}",
+   Style,
+   /*messUp=*/false);
+
+  verifyFormat("void foo() {\n"
+   "  #pragma omp simd\n"
+   "  for (...) {\n"
+   "#pragma omp simd\n"
+   "for (...) {\n"
+   "

[PATCH] D51650: Implement target_clones multiversioning

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In D51650#2418749 , @lebedev.ri wrote:

> In D51650#1305509 , @erichkeane 
> wrote:
>
>> Fix @rsmith s comments, rebase on the big CPUDispatch refactor.
>
> Ping. What's the status here?

I haven't looked at it since then :)  I thought it was about ready, and just 
needed review but this never got sufficient attention to get review.  It likely 
needs a rebase (and I'm not sure what other changes have been made in the 
meantime).


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

https://reviews.llvm.org/D51650

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

[PATCH] D92742: [clang] Add support for attribute 'swift_async'

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman added a comment.

LGTM aside from a minor nit.




Comment at: clang/include/clang/Basic/AttrDocs.td:4367
+``actuallyAsync:callThisAsync`` wouldn't have been imported as ``async`` if not
+for ``swift_async`` because it doesn't match the naming convention.
+

for `swift_async` because -> for the `swift_async` attribute because



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:6046
+  if (!SwiftAsyncAttr::ConvertStrToKind(II->getName(), Kind)) {
+S.Diag(AL.getLoc(), diag::err_swift_async_no_access) << AL << II;
+return;

FWIW, we usually use `warn_attribute_type_not_supported` for this diagnostic 
rather than coming up with a unique diagnostic per enumeration. (This sort of 
feels like something we should use tablegen to handle anyway.)

I don't insist on a change as your diagnostic is slightly better than what we 
usually use, but if you feel like making the change for consistency with other 
attribute behavior, that's fine by me as well.


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

https://reviews.llvm.org/D92742

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

[PATCH] D92761: [clang][AArch64][SVE] Avoid going through memory for VLAT <-> VLST casts

[Joe Ellis via Phabricator via cfe-commits]

joechrisellis created this revision.
joechrisellis added reviewers: peterwaller-arm, DavidTruby.
Herald added subscribers: psnobl, kristof.beyls, tschuett.
Herald added a reviewer: efriedma.
joechrisellis requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

This change makes use of the llvm.vector.extract intrinsic to avoid
going through memory when performing bitcasts between vector-length
agnostic types and vector-length specific types.

Depends on D91362 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D92761

Files:
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/test/CodeGen/aarch64-sve-acle-__ARM_FEATURE_SVE_VECTOR_OPERATORS.c
  clang/test/CodeGen/attr-arm-sve-vector-bits-bitcast.c
  clang/test/CodeGen/attr-arm-sve-vector-bits-call.c
  clang/test/CodeGen/attr-arm-sve-vector-bits-cast.c
  clang/test/CodeGen/attr-arm-sve-vector-bits-codegen.c
  clang/test/CodeGen/attr-arm-sve-vector-bits-globals.c
  llvm/include/llvm/IR/IRBuilder.h

Index: llvm/include/llvm/IR/IRBuilder.h
===
--- llvm/include/llvm/IR/IRBuilder.h
+++ llvm/include/llvm/IR/IRBuilder.h
@@ -919,6 +919,20 @@
 return CreateBinaryIntrinsic(Intrinsic::maximum, LHS, RHS, nullptr, Name);
   }
 
+  CallInst *CreateExtractVector(Type *DstType, Value *SrcVec, Value *Idx,
+const Twine &Name = "") {
+return CreateIntrinsic(Intrinsic::experimental_vector_extract,
+   {DstType, SrcVec->getType()}, {SrcVec, Idx}, nullptr,
+   Name);
+  }
+
+  CallInst *CreateInsertVector(Type *DstType, Value *SrcVec, Value *SubVec,
+   Value *Idx, const Twine &Name = "") {
+return CreateIntrinsic(Intrinsic::experimental_vector_insert,
+   {DstType, SubVec->getType()}, {SrcVec, SubVec, Idx},
+   nullptr, Name);
+  }
+
 private:
   /// Create a call to a masked intrinsic with given Id.
   CallInst *CreateMaskedIntrinsic(Intrinsic::ID Id, ArrayRef Ops,
Index: clang/test/CodeGen/attr-arm-sve-vector-bits-globals.c
===
--- clang/test/CodeGen/attr-arm-sve-vector-bits-globals.c
+++ clang/test/CodeGen/attr-arm-sve-vector-bits-globals.c
@@ -21,40 +21,28 @@
 
 // CHECK-128-LABEL: @write_global_i64(
 // CHECK-128-NEXT:  entry:
-// CHECK-128-NEXT:[[V_ADDR:%.*]] = alloca , align 16
-// CHECK-128-NEXT:store  [[V:%.*]], * [[V_ADDR]], align 16, [[TBAA6:!tbaa !.*]]
-// CHECK-128-NEXT:[[TMP0:%.*]] = bitcast * [[V_ADDR]] to <2 x i64>*
-// CHECK-128-NEXT:[[TMP1:%.*]] = load <2 x i64>, <2 x i64>* [[TMP0]], align 16, [[TBAA10:!tbaa !.*]]
-// CHECK-128-NEXT:store <2 x i64> [[TMP1]], <2 x i64>* @global_i64, align 16, [[TBAA10]]
+// CHECK-128-NEXT:[[CASTFIXEDSVE:%.*]] = call <2 x i64> @llvm.experimental.vector.extract.v2i64.nxv2i64( [[V:%.*]], i64 0)
+// CHECK-128-NEXT:store <2 x i64> [[CASTFIXEDSVE]], <2 x i64>* @global_i64, align 16, [[TBAA6:!tbaa !.*]]
 // CHECK-128-NEXT:ret void
 //
 // CHECK-512-LABEL: @write_global_i64(
 // CHECK-512-NEXT:  entry:
-// CHECK-512-NEXT:[[V_ADDR:%.*]] = alloca , align 16
-// CHECK-512-NEXT:store  [[V:%.*]], * [[V_ADDR]], align 16, [[TBAA6:!tbaa !.*]]
-// CHECK-512-NEXT:[[TMP0:%.*]] = bitcast * [[V_ADDR]] to <8 x i64>*
-// CHECK-512-NEXT:[[TMP1:%.*]] = load <8 x i64>, <8 x i64>* [[TMP0]], align 16, [[TBAA10:!tbaa !.*]]
-// CHECK-512-NEXT:store <8 x i64> [[TMP1]], <8 x i64>* @global_i64, align 16, [[TBAA10]]
+// CHECK-512-NEXT:[[CASTFIXEDSVE:%.*]] = call <8 x i64> @llvm.experimental.vector.extract.v8i64.nxv2i64( [[V:%.*]], i64 0)
+// CHECK-512-NEXT:store <8 x i64> [[CASTFIXEDSVE]], <8 x i64>* @global_i64, align 16, [[TBAA6:!tbaa !.*]]
 // CHECK-512-NEXT:ret void
 //
 void write_global_i64(svint64_t v) { global_i64 = v; }
 
 // CHECK-128-LABEL: @write_global_bf16(
 // CHECK-128-NEXT:  entry:
-// CHECK-128-NEXT:[[V_ADDR:%.*]] = alloca , align 16
-// CHECK-128-NEXT:store  [[V:%.*]], * [[V_ADDR]], align 16, [[TBAA11:!tbaa !.*]]
-// CHECK-128-NEXT:[[TMP0:%.*]] = bitcast * [[V_ADDR]] to <8 x bfloat>*
-// CHECK-128-NEXT:[[TMP1:%.*]] = load <8 x bfloat>, <8 x bfloat>* [[TMP0]], align 16, [[TBAA10]]
-// CHECK-128-NEXT:store <8 x bfloat> [[TMP1]], <8 x bfloat>* @global_bf16, align 16, [[TBAA10]]
+// CHECK-128-NEXT:[[CASTFIXEDSVE:%.*]] = call <8 x bfloat> @llvm.experimental.vector.extract.v8bf16.nxv8bf16( [[V:%.*]], i64 0)
+// CHECK-128-NEXT:store <8 x bfloat> [[CASTFIXEDSVE]], <8 x bfloat>* @global_bf16, align 16, [[TBAA6]]
 // CHECK-128-NEXT:ret void
 //
 // CHECK-512-LABEL: @write_global_bf16(
 // CHECK-512-NEXT:  entry:
-// CHECK-512-NEXT:[[V_ADDR:%.*]] = alloca , align 16
-// CHECK-512-NEXT:store  [[V:%.*]], * [[V_ADDR]], align 16, [[TBAA11:!tbaa !.*]]
-// CH

[PATCH] D92762: [clang][AArch64][SVE] Avoid going through memory for coerced VLST arguments

[Joe Ellis via Phabricator via cfe-commits]

joechrisellis created this revision.
joechrisellis added reviewers: peterwaller-arm, DavidTruby.
Herald added subscribers: psnobl, kristof.beyls, tschuett.
Herald added a reviewer: efriedma.
joechrisellis requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

VLST arguments are coerced to VLATs at the function boundary for
consistency with the VLAT ABI. They are then bitcast back to VLSTs in
the function prolog. Previously, this conversion is done through memory.
With the introduction of the llvm.vector.{insert,extract} intrinsic, we
can avoid going through memory here.

Depends on D92761 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D92762

Files:
  clang/lib/CodeGen/CGCall.cpp
  clang/test/CodeGen/aarch64-sve-acle-__ARM_FEATURE_SVE_VECTOR_OPERATORS.c
  clang/test/CodeGen/aarch64-sve-acle-__ARM_FEATURE_SVE_VECTOR_OPERATORS.cpp
  clang/test/CodeGen/attr-arm-sve-vector-bits-call.c
  clang/test/CodeGen/attr-arm-sve-vector-bits-cast.c

Index: clang/test/CodeGen/attr-arm-sve-vector-bits-cast.c
===
--- clang/test/CodeGen/attr-arm-sve-vector-bits-cast.c
+++ clang/test/CodeGen/attr-arm-sve-vector-bits-cast.c
@@ -13,11 +13,8 @@
 
 // CHECK-LABEL: @to_svint32_t(
 // CHECK-NEXT:  entry:
-// CHECK-NEXT:[[TYPE:%.*]] = alloca <16 x i32>, align 16
-// CHECK-NEXT:[[TMP0:%.*]] = bitcast <16 x i32>* [[TYPE]] to *
-// CHECK-NEXT:store  [[TYPE_COERCE:%.*]], * [[TMP0]], align 16
-// CHECK-NEXT:[[TYPE1:%.*]] = load <16 x i32>, <16 x i32>* [[TYPE]], align 16, [[TBAA6:!tbaa !.*]]
-// CHECK-NEXT:[[CASTSCALABLESVE:%.*]] = call  @llvm.experimental.vector.insert.nxv4i32.v16i32( undef, <16 x i32> [[TYPE1]], i64 0)
+// CHECK-NEXT:[[TYPE:%.*]] = call <16 x i32> @llvm.experimental.vector.extract.v16i32.nxv4i32( [[TYPE_COERCE:%.*]], i64 0)
+// CHECK-NEXT:[[CASTSCALABLESVE:%.*]] = call  @llvm.experimental.vector.insert.nxv4i32.v16i32( undef, <16 x i32> [[TYPE]], i64 0)
 // CHECK-NEXT:ret  [[CASTSCALABLESVE]]
 //
 svint32_t to_svint32_t(fixed_int32_t type) {
@@ -39,11 +36,8 @@
 
 // CHECK-LABEL: @to_svfloat64_t(
 // CHECK-NEXT:  entry:
-// CHECK-NEXT:[[TYPE:%.*]] = alloca <8 x double>, align 16
-// CHECK-NEXT:[[TMP0:%.*]] = bitcast <8 x double>* [[TYPE]] to *
-// CHECK-NEXT:store  [[TYPE_COERCE:%.*]], * [[TMP0]], align 16
-// CHECK-NEXT:[[TYPE1:%.*]] = load <8 x double>, <8 x double>* [[TYPE]], align 16, [[TBAA6]]
-// CHECK-NEXT:[[CASTSCALABLESVE:%.*]] = call  @llvm.experimental.vector.insert.nxv2f64.v8f64( undef, <8 x double> [[TYPE1]], i64 0)
+// CHECK-NEXT:[[TYPE:%.*]] = call <8 x double> @llvm.experimental.vector.extract.v8f64.nxv2f64( [[TYPE_COERCE:%.*]], i64 0)
+// CHECK-NEXT:[[CASTSCALABLESVE:%.*]] = call  @llvm.experimental.vector.insert.nxv2f64.v8f64( undef, <8 x double> [[TYPE]], i64 0)
 // CHECK-NEXT:ret  [[CASTSCALABLESVE]]
 //
 svfloat64_t to_svfloat64_t(fixed_float64_t type) {
@@ -65,15 +59,12 @@
 
 // CHECK-LABEL: @to_svbool_t(
 // CHECK-NEXT:  entry:
-// CHECK-NEXT:[[TYPE:%.*]] = alloca <8 x i8>, align 16
 // CHECK-NEXT:[[TYPE_ADDR:%.*]] = alloca <8 x i8>, align 16
-// CHECK-NEXT:[[TMP0:%.*]] = bitcast <8 x i8>* [[TYPE]] to *
-// CHECK-NEXT:store  [[TYPE_COERCE:%.*]], * [[TMP0]], align 16
-// CHECK-NEXT:[[TYPE1:%.*]] = load <8 x i8>, <8 x i8>* [[TYPE]], align 16, [[TBAA6]]
-// CHECK-NEXT:store <8 x i8> [[TYPE1]], <8 x i8>* [[TYPE_ADDR]], align 16, [[TBAA6]]
-// CHECK-NEXT:[[TMP1:%.*]] = bitcast <8 x i8>* [[TYPE_ADDR]] to *
-// CHECK-NEXT:[[TMP2:%.*]] = load , * [[TMP1]], align 16, [[TBAA6]]
-// CHECK-NEXT:ret  [[TMP2]]
+// CHECK-NEXT:[[TYPE:%.*]] = call <8 x i8> @llvm.experimental.vector.extract.v8i8.nxv16i1( [[TYPE_COERCE:%.*]], i64 0)
+// CHECK-NEXT:store <8 x i8> [[TYPE]], <8 x i8>* [[TYPE_ADDR]], align 16, [[TBAA6:!tbaa !.*]]
+// CHECK-NEXT:[[TMP0:%.*]] = bitcast <8 x i8>* [[TYPE_ADDR]] to *
+// CHECK-NEXT:[[TMP1:%.*]] = load , * [[TMP0]], align 16, [[TBAA6]]
+// CHECK-NEXT:ret  [[TMP1]]
 //
 svbool_t to_svbool_t(fixed_bool_t type) {
   return type;
@@ -130,11 +121,8 @@
 
 // CHECK-LABEL: @from_fixed_int32_t__to_gnu_int32_t(
 // CHECK-NEXT:  entry:
-// CHECK-NEXT:[[TYPE:%.*]] = alloca <16 x i32>, align 16
-// CHECK-NEXT:[[TMP0:%.*]] = bitcast <16 x i32>* [[TYPE]] to *
-// CHECK-NEXT:store  [[TYPE_COERCE:%.*]], * [[TMP0]], align 16
-// CHECK-NEXT:[[TYPE1:%.*]] = load <16 x i32>, <16 x i32>* [[TYPE]], align 16, [[TBAA6]]
-// CHECK-NEXT:store <16 x i32> [[TYPE1]], <16 x i32>* [[AGG_RESULT:%.*]], align 16, [[TBAA6]]
+// CHECK-NEXT:[[TYPE:%.*]] = call <16 x i32> @llvm.experimental.vector.extract.v16i32.nxv4i32( [[TYPE_COERCE:%.*]], i64 0)
+// CHECK-NEXT:store <16 x i32> [[TYPE]], <16 x i32>* [[AGG_RESULT:%.*]], align 16, [[TBAA6]]
 // CHECK-NEXT:ret void
 //
 gnu_int32_t from_fixed_int32_t__to_gnu_int32_t(fix

[PATCH] D92329: [PowerPC][Clang] Remove QPX support

[Jinsong Ji via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGb49b8f096c03: [PowerPC][Clang] Remove QPX support (authored 
by jsji).

Changed prior to commit:
  https://reviews.llvm.org/D92329?vs=308405&id=309908#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92329

Files:
  clang/docs/ClangCommandLineReference.rst
  clang/include/clang/Driver/Options.td
  clang/lib/AST/ASTContext.cpp
  clang/lib/Basic/Targets/PPC.h
  clang/lib/CodeGen/TargetInfo.cpp
  clang/test/CodeGen/ppc64-elf-abi.c
  clang/test/CodeGen/ppc64-qpx-vector.c
  clang/test/Driver/linux-ld.c
  clang/test/Driver/ppc-features.cpp
  clang/test/OpenMP/simd_metadata.c
  llvm/test/Transforms/LoopVectorize/PowerPC/vectorize-only-for-real.ll

Index: llvm/test/Transforms/LoopVectorize/PowerPC/vectorize-only-for-real.ll
===
--- llvm/test/Transforms/LoopVectorize/PowerPC/vectorize-only-for-real.ll
+++ llvm/test/Transforms/LoopVectorize/PowerPC/vectorize-only-for-real.ll
@@ -55,8 +55,8 @@
 
 declare signext i32 @bar(i32*, i32*) #2
 
-attributes #0 = { nounwind "target-cpu"="a2q" "target-features"="+qpx,-altivec,-bpermd,-crypto,-direct-move,-extdiv,-power8-vector,-vsx" }
+attributes #0 = { nounwind "target-features"="-altivec,-bpermd,-crypto,-direct-move,-extdiv,-power8-vector,-vsx" }
 attributes #1 = { argmemonly nounwind }
-attributes #2 = { "target-cpu"="a2q" "target-features"="+qpx,-altivec,-bpermd,-crypto,-direct-move,-extdiv,-power8-vector,-vsx" }
+attributes #2 = { "target-features"="-altivec,-bpermd,-crypto,-direct-move,-extdiv,-power8-vector,-vsx" }
 attributes #3 = { nounwind }
 
Index: clang/test/OpenMP/simd_metadata.c
===
--- clang/test/OpenMP/simd_metadata.c
+++ clang/test/OpenMP/simd_metadata.c
@@ -5,7 +5,6 @@
 // RUN: %clang_cc1 -fopenmp -triple i386-unknown-unknown -target-feature +avx -emit-llvm %s -o - | FileCheck %s -check-prefix=CHECK -check-prefix=X86-AVX
 // RUN: %clang_cc1 -fopenmp -triple i386-unknown-unknown -target-feature +avx512f -emit-llvm %s -o - | FileCheck %s -check-prefix=CHECK -check-prefix=X86-AVX512
 // RUN: %clang_cc1 -fopenmp -triple powerpc64-unknown-unknown -emit-llvm %s -o - | FileCheck %s -check-prefix=CHECK -check-prefix=PPC
-// RUN: %clang_cc1 -fopenmp -triple powerpc64-unknown-unknown -target-abi elfv1-qpx -emit-llvm %s -o - | FileCheck %s -check-prefix=CHECK -check-prefix=PPC-QPX
 
 // RUN: %clang_cc1 -fopenmp-simd -triple x86_64-unknown-unknown -emit-llvm %s -o - | FileCheck %s -check-prefix=CHECK -check-prefix=X86
 // RUN: %clang_cc1 -fopenmp-simd -triple x86_64-unknown-unknown -target-feature +avx -emit-llvm %s -o - | FileCheck %s -check-prefix=CHECK -check-prefix=X86-AVX
@@ -14,7 +13,6 @@
 // RUN: %clang_cc1 -fopenmp-simd -triple i386-unknown-unknown -target-feature +avx -emit-llvm %s -o - | FileCheck %s -check-prefix=CHECK -check-prefix=X86-AVX
 // RUN: %clang_cc1 -fopenmp-simd -triple i386-unknown-unknown -target-feature +avx512f -emit-llvm %s -o - | FileCheck %s -check-prefix=CHECK -check-prefix=X86-AVX512
 // RUN: %clang_cc1 -fopenmp-simd -triple powerpc64-unknown-unknown -emit-llvm %s -o - | FileCheck %s -check-prefix=CHECK -check-prefix=PPC
-// RUN: %clang_cc1 -fopenmp-simd -triple powerpc64-unknown-unknown -target-abi elfv1-qpx -emit-llvm %s -o - | FileCheck %s -check-prefix=CHECK -check-prefix=PPC-QPX
 
 void h1(float *c, float *a, double b[], int size)
 {
@@ -28,14 +26,12 @@
   // X86-AVX-NEXT:   call void @llvm.assume(i1 true) [ "align"(float* [[PTR5:%.*]], {{i64|i32}} 32) ]
   // X86-AVX512-NEXT:call void @llvm.assume(i1 true) [ "align"(float* [[PTR5:%.*]], {{i64|i32}} 64) ]
   // PPC-NEXT:   call void @llvm.assume(i1 true) [ "align"(float* [[PTR5:%.*]], {{i64|i32}} 16) ]
-  // PPC-QPX-NEXT:   call void @llvm.assume(i1 true) [ "align"(float* [[PTR5:%.*]], {{i64|i32}} 16) ]
   // CHECK-NEXT: load
 
   // X86-NEXT:   call void @llvm.assume(i1 true) [ "align"(double* [[PTR6:%.*]], {{i64|i32}} 16) ]
   // X86-AVX-NEXT:   call void @llvm.assume(i1 true) [ "align"(double* [[PTR6:%.*]], {{i64|i32}} 32) ]
   // X86-AVX512-NEXT:call void @llvm.assume(i1 true) [ "align"(double* [[PTR6:%.*]], {{i64|i32}} 64) ]
   // PPC-NEXT:   call void @llvm.assume(i1 true) [ "align"(double* [[PTR6:%.*]], {{i64|i32}} 16) ]
-  // PPC-QPX-NEXT:   call void @llvm.assume(i1 true) [ "align"(double* [[PTR6:%.*]], {{i64|i32}} 32) ]
   for (int i = 0; i < size; ++i) {
 c[i] = a[i] * a[i] + b[i] * b[t];
 ++t;
@@ -50,14 +46,12 @@
   // X86-AVX-NEXT:   call void @llvm.assume(i1 true) [ "align"(float* [[PTR5:%.*]], {{i64|i32}} 32) ]
   // X86-AVX512-NEXT:call void @llvm.assume(i1 true) [ "align"(float* [[PTR5:%.*]], {{i64|i32}} 64) ]
   // PPC-NEXT:   call void @llvm.assume(i1 true) [ "align"(float* [[PTR5:%.*]], {{i64|i32}} 16) ]
-  // PPC-QPX-NEXT:   call voi

[PATCH] D92764: [analyzer][StdLibraryFunctionsChecker] Make close and mmap to accept -1 as fd

[Gabor Marton via Phabricator via cfe-commits]

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

close:
It is quite often that users chose to call close even if the fd is
negative. Theoretically, it would be nicer to close only valid fds, but
in practice the implementations of close just returns with EBADF in case
of a non-valid fd param. So, we can eliminate many false positives if we
let close to take -1 as an fd. Other negative values are very unlikely,
because open and other fd factories return with -1 in case of failure.

mmap:
In the case of MAP_ANONYMOUS flag (which is supported e.g. in Linux) the
mapping is not backed by any file; its contents are initialized to zero.
The fd argument is ignored; however, some implementations require fd to
be -1 if MAP_ANONYMOUS (or MAP_ANON) is specified, and portable
applications should ensure this.
Consequently, we must allow -1 as the 4th arg.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D92764

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


Index: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -1672,7 +1672,7 @@
 addToFunctionSummaryMap("close", Signature(ArgTypes{IntTy}, 
RetType{IntTy}),
 Summary(NoEvalCall)
 .ArgConstraint(ArgumentCondition(
-0, WithinRange, Range(0, IntMax;
+0, WithinRange, Range(-1, IntMax;
 
 // long fpathconf(int fildes, int name);
 addToFunctionSummaryMap("fpathconf",
@@ -1746,7 +1746,7 @@
 Summary(NoEvalCall)
 .ArgConstraint(ArgumentCondition(1, WithinRange, Range(1, 
SizeMax)))
 .ArgConstraint(
-ArgumentCondition(4, WithinRange, Range(0, IntMax;
+ArgumentCondition(4, WithinRange, Range(-1, IntMax;
 
 // int pipe(int fildes[2]);
 addToFunctionSummaryMap(


Index: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -1672,7 +1672,7 @@
 addToFunctionSummaryMap("close", Signature(ArgTypes{IntTy}, RetType{IntTy}),
 Summary(NoEvalCall)
 .ArgConstraint(ArgumentCondition(
-0, WithinRange, Range(0, IntMax;
+0, WithinRange, Range(-1, IntMax;
 
 // long fpathconf(int fildes, int name);
 addToFunctionSummaryMap("fpathconf",
@@ -1746,7 +1746,7 @@
 Summary(NoEvalCall)
 .ArgConstraint(ArgumentCondition(1, WithinRange, Range(1, SizeMax)))
 .ArgConstraint(
-ArgumentCondition(4, WithinRange, Range(0, IntMax;
+ArgumentCondition(4, WithinRange, Range(-1, IntMax;
 
 // int pipe(int fildes[2]);
 addToFunctionSummaryMap(
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] b49b8f0 - [PowerPC][Clang] Remove QPX support

[Jinsong Ji via cfe-commits]

Author: Jinsong Ji
Date: 2020-12-07T10:15:39-05:00
New Revision: b49b8f096c0382da17d3203dfaa3f54d04a47d27

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

LOG: [PowerPC][Clang] Remove QPX support

Clean up QPX code in clang missed in https://reviews.llvm.org/D83915

Reviewed By: #powerpc, steven.zhang

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

Added: 


Modified: 
clang/docs/ClangCommandLineReference.rst
clang/include/clang/Driver/Options.td
clang/lib/AST/ASTContext.cpp
clang/lib/Basic/Targets/PPC.h
clang/lib/CodeGen/TargetInfo.cpp
clang/test/CodeGen/ppc64-elf-abi.c
clang/test/Driver/linux-ld.c
clang/test/Driver/ppc-features.cpp
clang/test/OpenMP/simd_metadata.c
llvm/test/Transforms/LoopVectorize/PowerPC/vectorize-only-for-real.ll

Removed: 
clang/test/CodeGen/ppc64-qpx-vector.c



diff  --git a/clang/docs/ClangCommandLineReference.rst 
b/clang/docs/ClangCommandLineReference.rst
index 3674f3a62695..ce510f335bd4 100644
--- a/clang/docs/ClangCommandLineReference.rst
+++ b/clang/docs/ClangCommandLineReference.rst
@@ -3169,8 +3169,6 @@ PowerPC
 
 .. option:: -mpower9-vector, -mno-power9-vector
 
-.. option:: -mqpx, -mno-qpx
-
 .. option:: -msecure-plt
 
 .. option:: -mspe, -mno-spe

diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index b58f5cbc63d0..6480d6e80293 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -2639,8 +2639,6 @@ def mno_mfocrf : Flag<["-"], "mno-mfocrf">, 
Group;
 def mno_mfcrf : Flag<["-"], "mno-mfcrf">, Alias;
 def mpopcntd : Flag<["-"], "mpopcntd">, Group;
 def mno_popcntd : Flag<["-"], "mno-popcntd">, Group;
-def mqpx : Flag<["-"], "mqpx">, Group;
-def mno_qpx : Flag<["-"], "mno-qpx">, Group;
 def mcrbits : Flag<["-"], "mcrbits">, Group;
 def mno_crbits : Flag<["-"], "mno-crbits">, Group;
 def minvariant_function_descriptors :

diff  --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index dacb9679c493..c52369cd8a02 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -2360,12 +2360,6 @@ unsigned ASTContext::getTypeUnadjustedAlign(const Type 
*T) const {
 
 unsigned ASTContext::getOpenMPDefaultSimdAlign(QualType T) const {
   unsigned SimdAlign = getTargetInfo().getSimdDefaultAlign();
-  // Target ppc64 with QPX: simd default alignment for pointer to double is 32.
-  if ((getTargetInfo().getTriple().getArch() == llvm::Triple::ppc64 ||
-   getTargetInfo().getTriple().getArch() == llvm::Triple::ppc64le) &&
-  getTargetInfo().getABI() == "elfv1-qpx" &&
-  T->isSpecificBuiltinType(BuiltinType::Double))
-SimdAlign = 256;
   return SimdAlign;
 }
 

diff  --git a/clang/lib/Basic/Targets/PPC.h b/clang/lib/Basic/Targets/PPC.h
index ad754462370f..a4677cd067f7 100644
--- a/clang/lib/Basic/Targets/PPC.h
+++ b/clang/lib/Basic/Targets/PPC.h
@@ -438,7 +438,7 @@ class LLVM_LIBRARY_VISIBILITY PPC64TargetInfo : public 
PPCTargetInfo {
 
   // PPC64 Linux-specific ABI options.
   bool setABI(const std::string &Name) override {
-if (Name == "elfv1" || Name == "elfv1-qpx" || Name == "elfv2") {
+if (Name == "elfv1" || Name == "elfv2") {
   ABI = Name;
   return true;
 }

diff  --git a/clang/lib/CodeGen/TargetInfo.cpp 
b/clang/lib/CodeGen/TargetInfo.cpp
index 4815266371bc..7213f7864d43 100644
--- a/clang/lib/CodeGen/TargetInfo.cpp
+++ b/clang/lib/CodeGen/TargetInfo.cpp
@@ -4885,42 +4885,12 @@ class PPC64_SVR4_ABIInfo : public SwiftABIInfo {
 private:
   static const unsigned GPRBits = 64;
   ABIKind Kind;
-  bool HasQPX;
   bool IsSoftFloatABI;
 
-  // A vector of float or double will be promoted to <4 x f32> or <4 x f64> and
-  // will be passed in a QPX register.
-  bool IsQPXVectorTy(const Type *Ty) const {
-if (!HasQPX)
-  return false;
-
-if (const VectorType *VT = Ty->getAs()) {
-  unsigned NumElements = VT->getNumElements();
-  if (NumElements == 1)
-return false;
-
-  if (VT->getElementType()->isSpecificBuiltinType(BuiltinType::Double)) {
-if (getContext().getTypeSize(Ty) <= 256)
-  return true;
-  } else if (VT->getElementType()->
-   isSpecificBuiltinType(BuiltinType::Float)) {
-if (getContext().getTypeSize(Ty) <= 128)
-  return true;
-  }
-}
-
-return false;
-  }
-
-  bool IsQPXVectorTy(QualType Ty) const {
-return IsQPXVectorTy(Ty.getTypePtr());
-  }
-
 public:
-  PPC64_SVR4_ABIInfo(CodeGen::CodeGenTypes &CGT, ABIKind Kind, bool HasQPX,
+  PPC64_SVR4_ABIInfo(CodeGen::CodeGenTypes &CGT, ABIKind Kind,
  bool SoftFloatABI)
-  : SwiftABIInfo(CGT), Kind(Kind), HasQPX(HasQPX),
-IsSoftFloatABI(SoftFloatABI) {}
+  : SwiftABII

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

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

kpn added inline comments.



Comment at: clang/lib/CodeGen/CGExprScalar.cpp:2992
 // floating point environment in the loop.
+//XXX true?
 llvm::BasicBlock *startBB = Builder.GetInsertBlock();

mibintc wrote:
> did you mean to leave this here? (blame shows the fixme comment dates from 
> 2012)
I was hoping someone knew what "floating point environment" is relevant here. 
From reading the commit message it doesn't sound like it matters to us, but I 
thought I'd flag it anyway.

No, I wasn't planning on committing with this comment.



Comment at: clang/lib/CodeGen/CGExprScalar.cpp:3006
 
+  CodeGenFunction::CGFPOptionsRAII FPOptsRAII(CGF, OpInfo.FPFeatures);
   SourceLocation Loc = E->getExprLoc();

mibintc wrote:
> What's the rule to follow about when we need to FPOptsRAII? 
It is used on the border between code that has the AST node and code that 
doesn't. If any code below this point might use the constrained floating point 
intrinsics then the FPOptsRAII is needed.

Sometimes this border is at a call to the IRBuilder. Sometimes it's buried 
elsewhere. The hope is that by having the location be defined there we can at 
some point audit to verify we have it everywhere we should.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92596

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

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

[Melanie Blower via Phabricator via cfe-commits]

mibintc added inline comments.



Comment at: clang/lib/CodeGen/CGExprScalar.cpp:3006
 
+  CodeGenFunction::CGFPOptionsRAII FPOptsRAII(CGF, OpInfo.FPFeatures);
   SourceLocation Loc = E->getExprLoc();

kpn wrote:
> mibintc wrote:
> > What's the rule to follow about when we need to FPOptsRAII? 
> It is used on the border between code that has the AST node and code that 
> doesn't. If any code below this point might use the constrained floating 
> point intrinsics then the FPOptsRAII is needed.
> 
> Sometimes this border is at a call to the IRBuilder. Sometimes it's buried 
> elsewhere. The hope is that by having the location be defined there we can at 
> some point audit to verify we have it everywhere we should.
Oh, that doesn't sound very bug-proof.  Do you mind pointing out 2 different 
instances, one "AST node" and one "buried elsewhere"?  Or there's probably a 
code review I should read to find this?  Thank you


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92596

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

[PATCH] D92753: [clang-format] Add IndentPragma style to eliminate common clang-format off scenario

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added a comment.

Maybe it would be judicious to add a FIXME to `test::messUp` for the pragmas? 
Or fix it in this patch?




Comment at: clang/unittests/Format/FormatTest.cpp:17641-17651
+  verifyFormat("void foo() {\n"
+   "  #pragma omp simd\n"
+   "  for (...) {\n"
+   "#pragma omp simd\n"
+   "for (...) {\n"
+   "}\n"
+   "  }\n"

Actually I'd rather see a test were you mess up spaces manually (adding or 
removing spaces on pragma lines).
So instead of `verifyFormat(formattedCode)`, I'd see `EXPECT_EQ(formattedCode, 
format(lightly-messed-up-code, Style)`.


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

https://reviews.llvm.org/D92753

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

[PATCH] D92764: [analyzer][StdLibraryFunctionsChecker] Make close and mmap to accept -1 as fd

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

steakhal added a comment.

LGTM besides my inline comment.




Comment at: 
clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:1737
 .ArgConstraint(
 ArgumentCondition(4, WithinRange, Range(0, IntMax;
 

`mmap` should have the same conditions, am I right?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92764

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

[PATCH] D92727: [CodeGen][MSan] Don't use offsets of zero-sized fields

[Matt Morehouse via Phabricator via cfe-commits]

morehouse accepted this revision.
morehouse 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/D92727/new/

https://reviews.llvm.org/D92727

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

[PATCH] D92658: [libTooling] Add `describe` stencil for formatting AST nodes for diagnostics.

[Yitzhak Mandelbaum 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 rGe6bc4a71e345: [libTooling] Add `describe` combinator for 
formatting AST nodes for diagnostics. (authored by ymandel).

Changed prior to commit:
  https://reviews.llvm.org/D92658?vs=309590&id=309922#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92658

Files:
  clang/include/clang/Tooling/Transformer/Stencil.h
  clang/lib/Tooling/Transformer/Stencil.cpp
  clang/unittests/Tooling/StencilTest.cpp

Index: clang/unittests/Tooling/StencilTest.cpp
===
--- clang/unittests/Tooling/StencilTest.cpp
+++ clang/unittests/Tooling/StencilTest.cpp
@@ -30,7 +30,9 @@
 
 // Create a valid translation-unit from a statement.
 static std::string wrapSnippet(StringRef StatementCode) {
-  return ("struct S { int field; }; auto stencil_test_snippet = []{" +
+  return ("namespace N { class C {}; } "
+  "namespace { class AnonC {}; } "
+  "struct S { int field; }; auto stencil_test_snippet = []{" +
   StatementCode + "};")
   .str();
 }
@@ -55,14 +57,14 @@
 // `StatementCode` may contain other statements not described by `Matcher`.
 static llvm::Optional matchStmt(StringRef StatementCode,
StatementMatcher Matcher) {
-  auto AstUnit = tooling::buildASTFromCode(wrapSnippet(StatementCode));
+  auto AstUnit = tooling::buildASTFromCodeWithArgs(wrapSnippet(StatementCode),
+   {"-Wno-unused-value"});
   if (AstUnit == nullptr) {
 ADD_FAILURE() << "AST construction failed";
 return llvm::None;
   }
   ASTContext &Context = AstUnit->getASTContext();
-  auto Matches = ast_matchers::match(
-  traverse(ast_type_traits::TK_AsIs, wrapMatcher(Matcher)), Context);
+  auto Matches = ast_matchers::match(wrapMatcher(Matcher), Context);
   // We expect a single, exact match for the statement.
   if (Matches.size() != 1) {
 ADD_FAILURE() << "Wrong number of matches: " << Matches.size();
@@ -365,6 +367,66 @@
   EXPECT_THAT_EXPECTED(Stencil->eval(StmtMatch->Result), HasValue("field"));
 }
 
+TEST_F(StencilTest, DescribeType) {
+  std::string Snippet = "int *x; x;";
+  std::string Expected = "int *";
+  auto StmtMatch =
+  matchStmt(Snippet, declRefExpr(hasType(qualType().bind("type";
+  ASSERT_TRUE(StmtMatch);
+  EXPECT_THAT_EXPECTED(describe("type")->eval(StmtMatch->Result),
+   HasValue(std::string(Expected)));
+}
+
+TEST_F(StencilTest, DescribeSugaredType) {
+  std::string Snippet = "using Ty = int; Ty *x; x;";
+  std::string Expected = "Ty *";
+  auto StmtMatch =
+  matchStmt(Snippet, declRefExpr(hasType(qualType().bind("type";
+  ASSERT_TRUE(StmtMatch);
+  EXPECT_THAT_EXPECTED(describe("type")->eval(StmtMatch->Result),
+   HasValue(std::string(Expected)));
+}
+
+TEST_F(StencilTest, DescribeDeclType) {
+  std::string Snippet = "S s; s;";
+  std::string Expected = "S";
+  auto StmtMatch =
+  matchStmt(Snippet, declRefExpr(hasType(qualType().bind("type";
+  ASSERT_TRUE(StmtMatch);
+  EXPECT_THAT_EXPECTED(describe("type")->eval(StmtMatch->Result),
+   HasValue(std::string(Expected)));
+}
+
+TEST_F(StencilTest, DescribeQualifiedType) {
+  std::string Snippet = "N::C c; c;";
+  std::string Expected = "N::C";
+  auto StmtMatch =
+  matchStmt(Snippet, declRefExpr(hasType(qualType().bind("type";
+  ASSERT_TRUE(StmtMatch);
+  EXPECT_THAT_EXPECTED(describe("type")->eval(StmtMatch->Result),
+   HasValue(std::string(Expected)));
+}
+
+TEST_F(StencilTest, DescribeUnqualifiedType) {
+  std::string Snippet = "using N::C; C c; c;";
+  std::string Expected = "N::C";
+  auto StmtMatch =
+  matchStmt(Snippet, declRefExpr(hasType(qualType().bind("type";
+  ASSERT_TRUE(StmtMatch);
+  EXPECT_THAT_EXPECTED(describe("type")->eval(StmtMatch->Result),
+   HasValue(std::string(Expected)));
+}
+
+TEST_F(StencilTest, DescribeAnonNamespaceType) {
+  std::string Snippet = "AnonC c; c;";
+  std::string Expected = "(anonymous namespace)::AnonC";
+  auto StmtMatch =
+  matchStmt(Snippet, declRefExpr(hasType(qualType().bind("type";
+  ASSERT_TRUE(StmtMatch);
+  EXPECT_THAT_EXPECTED(describe("type")->eval(StmtMatch->Result),
+   HasValue(std::string(Expected)));
+}
+
 TEST_F(StencilTest, RunOp) {
   StringRef Id = "id";
   auto SimpleFn = [Id](const MatchResult &R) {
@@ -436,6 +498,12 @@
   });
 }
 
+// The `StencilToStringTest` tests verify that the string representation of the
+// stencil combinator matches (as best possible) the spelling of the
+// combinator's construction.  Exceptions include those combinators that have no
+// explicit spelling (like raw text) and those supporting non-pr

[clang] e6bc4a7 - [libTooling] Add `describe` combinator for formatting AST nodes for diagnostics.

[Yitzhak Mandelbaum via cfe-commits]

Author: Yitzhak Mandelbaum
Date: 2020-12-07T16:08:05Z
New Revision: e6bc4a71e3450d7230205683f63d6e22fbf9bf05

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

LOG: [libTooling] Add `describe` combinator for formatting AST nodes for 
diagnostics.

This new stencil combinator is intended for use in diagnostics and the like.

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

Added: 


Modified: 
clang/include/clang/Tooling/Transformer/Stencil.h
clang/lib/Tooling/Transformer/Stencil.cpp
clang/unittests/Tooling/StencilTest.cpp

Removed: 




diff  --git a/clang/include/clang/Tooling/Transformer/Stencil.h 
b/clang/include/clang/Tooling/Transformer/Stencil.h
index 1b50a670f70b..b729c826c808 100644
--- a/clang/include/clang/Tooling/Transformer/Stencil.h
+++ b/clang/include/clang/Tooling/Transformer/Stencil.h
@@ -123,6 +123,15 @@ inline Stencil ifBound(llvm::StringRef Id, llvm::StringRef 
TrueText,
 /// Stencil.  This supports user-defined extensions to the \c Stencil language.
 Stencil run(MatchConsumer C);
 
+/// Produces a human-readable rendering of the node bound to `Id`, suitable for
+/// diagnostics and debugging. This operator can be applied to any node, but is
+/// targeted at those whose source cannot be printed directly, including:
+///
+/// * Types. represented based on their structure. Note that namespace
+///   qualifiers are always printed, with the anonymous namespace represented
+///   explicitly. No desugaring or canonicalization is applied.
+Stencil describe(llvm::StringRef Id);
+
 /// For debug use only; semantics are not guaranteed.
 ///
 /// \returns the string resulting from calling the node's print() method.

diff  --git a/clang/lib/Tooling/Transformer/Stencil.cpp 
b/clang/lib/Tooling/Transformer/Stencil.cpp
index 2670bf7adabf..56f145393691 100644
--- a/clang/lib/Tooling/Transformer/Stencil.cpp
+++ b/clang/lib/Tooling/Transformer/Stencil.cpp
@@ -63,6 +63,7 @@ enum class UnaryNodeOperator {
   MaybeDeref,
   AddressOf,
   MaybeAddressOf,
+  Describe,
 };
 
 // Generic container for stencil operations with a (single) node-id argument.
@@ -133,6 +134,9 @@ std::string toStringData(const UnaryOperationData &Data) {
   case UnaryNodeOperator::MaybeAddressOf:
 OpName = "maybeAddressOf";
 break;
+  case UnaryNodeOperator::Describe:
+OpName = "describe";
+break;
   }
   return (OpName + "(\"" + Data.Id + "\")").str();
 }
@@ -174,11 +178,11 @@ Error evalData(const RawTextData &Data, const 
MatchFinder::MatchResult &,
   return Error::success();
 }
 
-Error evalData(const DebugPrintNodeData &Data,
-   const MatchFinder::MatchResult &Match, std::string *Result) {
+static Error printNode(StringRef Id, const MatchFinder::MatchResult &Match,
+   std::string *Result) {
   std::string Output;
   llvm::raw_string_ostream Os(Output);
-  auto NodeOrErr = getNode(Match.Nodes, Data.Id);
+  auto NodeOrErr = getNode(Match.Nodes, Id);
   if (auto Err = NodeOrErr.takeError())
 return Err;
   NodeOrErr->print(Os, PrintingPolicy(Match.Context->getLangOpts()));
@@ -186,8 +190,18 @@ Error evalData(const DebugPrintNodeData &Data,
   return Error::success();
 }
 
+Error evalData(const DebugPrintNodeData &Data,
+   const MatchFinder::MatchResult &Match, std::string *Result) {
+  return printNode(Data.Id, Match, Result);
+}
+
 Error evalData(const UnaryOperationData &Data,
const MatchFinder::MatchResult &Match, std::string *Result) {
+  // The `Describe` operation can be applied to any node, not just expressions,
+  // so it is handled here, separately.
+  if (Data.Op == UnaryNodeOperator::Describe)
+return printNode(Data.Id, Match, Result);
+
   const auto *E = Match.Nodes.getNodeAs(Data.Id);
   if (E == nullptr)
 return llvm::make_error(
@@ -217,6 +231,8 @@ Error evalData(const UnaryOperationData &Data,
 }
 Source = tooling::buildAddressOf(*E, *Match.Context);
 break;
+  case UnaryNodeOperator::Describe:
+llvm_unreachable("This case is handled at the start of the function");
   }
   if (!Source)
 return llvm::make_error(
@@ -359,6 +375,11 @@ Stencil transformer::maybeAddressOf(llvm::StringRef 
ExprId) {
   UnaryNodeOperator::MaybeAddressOf, std::string(ExprId));
 }
 
+Stencil transformer::describe(StringRef Id) {
+  return std::make_shared>(
+  UnaryNodeOperator::Describe, std::string(Id));
+}
+
 Stencil transformer::access(StringRef BaseId, Stencil Member) {
   return std::make_shared>(BaseId, std::move(Member));
 }

diff  --git a/clang/unittests/Tooling/StencilTest.cpp 
b/clang/unittests/Tooling/StencilTest.cpp
index c843e33dd0da..56e81431e558 100644
--- a/clang/unittests/Tooling/StencilTest.cpp
+++ b/clang/unittests/Tooling/StencilTest.cpp
@@ -30,7 +30,9 @@ using 

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

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

kpn added inline comments.



Comment at: clang/lib/CodeGen/CGExprScalar.cpp:3006
 
+  CodeGenFunction::CGFPOptionsRAII FPOptsRAII(CGF, OpInfo.FPFeatures);
   SourceLocation Loc = E->getExprLoc();

mibintc wrote:
> kpn wrote:
> > mibintc wrote:
> > > What's the rule to follow about when we need to FPOptsRAII? 
> > It is used on the border between code that has the AST node and code that 
> > doesn't. If any code below this point might use the constrained floating 
> > point intrinsics then the FPOptsRAII is needed.
> > 
> > Sometimes this border is at a call to the IRBuilder. Sometimes it's buried 
> > elsewhere. The hope is that by having the location be defined there we can 
> > at some point audit to verify we have it everywhere we should.
> Oh, that doesn't sound very bug-proof.  Do you mind pointing out 2 different 
> instances, one "AST node" and one "buried elsewhere"?  Or there's probably a 
> code review I should read to find this?  Thank you
Agreed, but the approach of wrapping calls to the IRBuilder isn't very 
bug-proof either. And it requires threading strictfp info throughout code that 
otherwise doesn't support carrying around info currently. So it would be 
invasive and hard to audit.

This patch is an example of code that has the AST node (the Expr*) and is 
calling the IRBuilder. For "buried" examples see D88913.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92596

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

[PATCH] D92728: [NFC][MSan] Round up OffsetPtr in PoisonMembersgetFieldOffset(layoutStartOffset) for current calleds is expected topoint to the first trivial field or the one which follows non-trivial

[Matt Morehouse via Phabricator via cfe-commits]

morehouse added inline comments.



Comment at: clang/lib/CodeGen/CGClass.cpp:1742
+  Context.toCharUnitsFromBits(Layout.getFieldOffset(layoutStartOffset) 
+
+  Context.getCharWidth() - 1);
+  llvm::ConstantInt *OffsetSizePtr =

How does this interact with bit fields?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92728

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

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

[Melanie Blower via Phabricator via cfe-commits]

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

Thanks for the additional info @kpn


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92596

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

[PATCH] D92753: [clang-format] Add IndentPragma style to eliminate common clang-format off scenario

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay updated this revision to Diff 309926.
MyDeveloperDay added a comment.

Allow the tests to messUp() the code, turns out its some sort of interaction 
ONLY with the BeforeHash case (last test)


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

https://reviews.llvm.org/D92753

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/ContinuationIndenter.cpp
  clang/lib/Format/Format.cpp
  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
@@ -14116,6 +14116,7 @@
   CHECK_PARSE_BOOL(IndentCaseLabels);
   CHECK_PARSE_BOOL(IndentCaseBlocks);
   CHECK_PARSE_BOOL(IndentGotoLabels);
+  CHECK_PARSE_BOOL(IndentPragmas);
   CHECK_PARSE_BOOL(IndentRequires);
   CHECK_PARSE_BOOL(IndentWrappedFunctionNames);
   CHECK_PARSE_BOOL(KeepEmptyLinesAtTheStartOfBlocks);
@@ -17562,6 +17563,129 @@
"struct constant;",
Style);
 }
+
+TEST_F(FormatTest, IndentPragmas) {
+  FormatStyle Style = getLLVMStyle();
+  Style.IndentPPDirectives = FormatStyle::PPDIS_None;
+
+  Style.IndentPragmas = false;
+  verifyFormat("#pragma once", Style);
+  verifyFormat("#pragma omp simd\n"
+   "for (int i = 0; i < 10; i++) {\n"
+   "}",
+   Style);
+  verifyFormat("void foo() {\n"
+   "#pragma omp simd\n"
+   "  for (int i = 0; i < 10; i++) {\n"
+   "  }\n"
+   "}",
+   Style);
+  verifyFormat("void foo() {\n"
+   "// outer loop\n"
+   "#pragma omp simd\n"
+   "  for (int k = 0; k < 10; k++) {\n"
+   "// inner loop\n"
+   "#pragma omp simd\n"
+   "for (int j = 0; j < 10; j++) {\n"
+   "}\n"
+   "  }\n"
+   "}",
+   Style);
+
+  verifyFormat("void foo() {\n"
+   "// outer loop\n"
+   "#if 1\n"
+   "#pragma omp simd\n"
+   "  for (int k = 0; k < 10; k++) {\n"
+   "// inner loop\n"
+   "#pragma omp simd\n"
+   "for (int j = 0; j < 10; j++) {\n"
+   "}\n"
+   "  }\n"
+   "#endif\n"
+   "}",
+   Style);
+
+  Style.IndentPragmas = true;
+  verifyFormat("#pragma once", Style);
+  verifyFormat("#pragma omp simd\n"
+   "for (int i = 0; i < 10; i++) {\n"
+   "}",
+   Style);
+  verifyFormat("void foo() {\n"
+   "  #pragma omp simd\n"
+   "  for (int i = 0; i < 10; i++) {\n"
+   "  }\n"
+   "}",
+   Style);
+  verifyFormat("void foo() {\n"
+   "  #pragma omp simd\n"
+   "  for (int i = 0; i < 10; i++) {\n"
+   "#pragma omp simd\n"
+   "for (int j = 0; j < 10; j++) {\n"
+   "}\n"
+   "  }\n"
+   "}",
+   Style);
+
+  verifyFormat("void foo() {\n"
+   "  #pragma omp simd\n"
+   "  for (...) {\n"
+   "#pragma omp simd\n"
+   "for (...) {\n"
+   "}\n"
+   "  }\n"
+   "}",
+   Style);
+
+  Style.IndentPPDirectives = FormatStyle::PPDIS_AfterHash;
+
+  verifyFormat("void foo() {\n"
+   "# pragma omp simd\n"
+   "  for (int i = 0; i < 10; i++) {\n"
+   "#   pragma omp simd\n"
+   "for (int j = 0; j < 10; j++) {\n"
+   "}\n"
+   "  }\n"
+   "}",
+   Style);
+
+  verifyFormat("void foo() {\n"
+   "#if 1\n"
+   "# pragma omp simd\n"
+   "  for (int k = 0; k < 10; k++) {\n"
+   "#   pragma omp simd\n"
+   "for (int j = 0; j < 10; j++) {\n"
+   "}\n"
+   "  }\n"
+   "#endif\n"
+   "}",
+   Style);
+
+  Style.IndentPPDirectives = FormatStyle::PPDIS_BeforeHash;
+  EXPECT_EQ("void foo() {\n"
+"#if 1\n"
+"  #pragma omp simd\n"
+"  for (int k = 0; k < 10; k++) {\n"
+"#pragma omp simd\n"
+"for (int j = 0; j < 10; j++) {\n"
+"}\n"
+"  }\n"
+"#endif\n"
+"}",
+format("void foo() {\n"
+   "#if 1\n"
+   "  #pragma omp simd\n"
+   "  for (int k = 0; k < 10; k++) {\n"
+   "#pragma omp simd\n"
+   "for (int j = 0; j < 10; j++) {\n"
+   " 

[PATCH] D92764: [analyzer][StdLibraryFunctionsChecker] Make close and mmap to accept -1 as fd

[Gabor Marton via Phabricator via cfe-commits]

martong updated this revision to Diff 309931.
martong marked an inline comment as done.
martong added a comment.

- Use -1 for mmap too


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92764

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


Index: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -1672,7 +1672,7 @@
 addToFunctionSummaryMap("close", Signature(ArgTypes{IntTy}, 
RetType{IntTy}),
 Summary(NoEvalCall)
 .ArgConstraint(ArgumentCondition(
-0, WithinRange, Range(0, IntMax;
+0, WithinRange, Range(-1, IntMax;
 
 // long fpathconf(int fildes, int name);
 addToFunctionSummaryMap("fpathconf",
@@ -1734,7 +1734,7 @@
 Summary(NoEvalCall)
 .ArgConstraint(ArgumentCondition(1, WithinRange, Range(1, 
SizeMax)))
 .ArgConstraint(
-ArgumentCondition(4, WithinRange, Range(0, IntMax;
+ArgumentCondition(4, WithinRange, Range(-1, IntMax;
 
 Optional Off64_tTy = lookupTy("off64_t");
 // void *mmap64(void *addr, size_t length, int prot, int flags, int fd,
@@ -1746,7 +1746,7 @@
 Summary(NoEvalCall)
 .ArgConstraint(ArgumentCondition(1, WithinRange, Range(1, 
SizeMax)))
 .ArgConstraint(
-ArgumentCondition(4, WithinRange, Range(0, IntMax;
+ArgumentCondition(4, WithinRange, Range(-1, IntMax;
 
 // int pipe(int fildes[2]);
 addToFunctionSummaryMap(


Index: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -1672,7 +1672,7 @@
 addToFunctionSummaryMap("close", Signature(ArgTypes{IntTy}, RetType{IntTy}),
 Summary(NoEvalCall)
 .ArgConstraint(ArgumentCondition(
-0, WithinRange, Range(0, IntMax;
+0, WithinRange, Range(-1, IntMax;
 
 // long fpathconf(int fildes, int name);
 addToFunctionSummaryMap("fpathconf",
@@ -1734,7 +1734,7 @@
 Summary(NoEvalCall)
 .ArgConstraint(ArgumentCondition(1, WithinRange, Range(1, SizeMax)))
 .ArgConstraint(
-ArgumentCondition(4, WithinRange, Range(0, IntMax;
+ArgumentCondition(4, WithinRange, Range(-1, IntMax;
 
 Optional Off64_tTy = lookupTy("off64_t");
 // void *mmap64(void *addr, size_t length, int prot, int flags, int fd,
@@ -1746,7 +1746,7 @@
 Summary(NoEvalCall)
 .ArgConstraint(ArgumentCondition(1, WithinRange, Range(1, SizeMax)))
 .ArgConstraint(
-ArgumentCondition(4, WithinRange, Range(0, IntMax;
+ArgumentCondition(4, WithinRange, Range(-1, IntMax;
 
 // int pipe(int fildes[2]);
 addToFunctionSummaryMap(
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D92764: [analyzer][StdLibraryFunctionsChecker] Make close and mmap to accept -1 as fd

[Gabor Marton via Phabricator via cfe-commits]

martong added inline comments.



Comment at: 
clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:1737
 .ArgConstraint(
 ArgumentCondition(4, WithinRange, Range(0, IntMax;
 

steakhal wrote:
> `mmap` should have the same conditions, am I right?
Yep, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92764

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