Input Needed: Teams for Pull Request Subscriptions

[ldionne]

Thanks @tstellar !

Here are the teams for libc++, libc++abi and libunwind:

@pr-subscribers-libcxx

libcxx/
runtimes/

@pr-subscribers-libcxxabi

libcxxabi/
runtimes/

@pr-subscribers-libunwind

libunwind/
runtimes/

I know we chatted about those previously and I gave you slightly different paths, but those are the right ones to use.

[MaskRay]

@cor3ntin

How about

pr-subscribers-clang
clang/**

We are probably going to need more fine grained control for clang-format, tools, analysis/cfg, ast matchers, etc… (codegen was already mentioned) but an all compasing group does seem useful

Yes, this umbrella team is useful for folks who want to know everything in clang/. If GitHub CODEOWNERS file faithfully provides gitignore style features, we can use this command to check whether a pattern works. Note that ** at the end can be omitted:

% grep /clang/\$ .gitignore  # add a pattern locally for experiments
/clang/
% git check-ignore -v --no-index clang/lib/Basic/Attributes.cpp
.gitignore:74:/clang/   clang/lib/Basic/Attributes.cpp

[MaskRay]

@jh7370

@jdenny-ornl

llvm/docs/CommandGuide/FileCheck.rst

@MaskRay, this made me realise that the binary utils one should include the corresponding documentation too (including documentaiton for aliased executables like llvm-addr2line or llvm-strip).

Added these:

/llvm/docs/CommandGuide/llvm-*
/llvm/include/llvm/ObjCopy/
/llvm/lib/BinaryFormat/
/llvm/lib/DebugInfo/Symbolize/
/llvm/lib/ObjCopy/
/llvm/lib/Object/
/llvm/test/Object/

Certain files are not related to binary utilities, but most are related. Having a high recall rate is probably more important.

@MaskRay, this made me realise that the binary utils one should include the corresponding documentation too (including documentaiton for aliased executables like llvm-addr2line or llvm-strip).

For doc, I added llvm/docs/CommandGuide/llvm-* for now. It’s probably too cumbersome to add every individual llvm-*.rst file… (and there is no brace expansion)

[MaskRay]

@jh7370

I’m tempted to suggest a yaml2obj/obj2yaml team too, but that should probably be independent. WDYT?

How about the following?

pr-subscribers-objectyaml

/llvm/include/llvm/ObjectYAML/
/llvm/lib/ObjectYAML/
/llvm/test/tools/obj2yaml/
/llvm/test/tools/yaml2obj/
/llvm/tools/obj2yaml/
/llvm/tools/yaml2obj/

[MaskRay]

@efriedma-quic

Can individual files be added to a team? I’d like, for example, an “clang AArch64” team that covers various bits of clang, but none of those bits are in a distinct directory. Similarly, a “SCEV” team that covers the core bits of ScalarEvolution/ScalarEvolutionExpander.

This makes me feel that we need a guideline or example what granularity we want.
The fragmentation seems more severe for backend teams.

For the AArch64 backend, there are miscellaneous files like

clang/include/clang/Basic/BuiltinsAArch64*
clang/lib/Driver/ToolChains/Arch/AArch64.*
clang/lib/CodeGen/Targets/AArch64.cpp
clang/lib/Basic/Targets/AArch64.*
llvm/docs/AArch64SME.rst
llvm/include/llvm/BinaryFormat/ELFRelocs/AArch64.def
llvm/include/llvm/IR/IntrinsicsAArch64.td
llvm/include/llvm/TargetParser/AArch64TargetParser.h

and many AArch64/ directories under a very specific test directory, e.g. llvm/test/Transforms/LoopUnroll/AArch64/ and llvm/test/LTO/AArch64/

@TNorthover @sjoerdmeijer @statham-arm

edit: I suggested the following on https://github.com/llvm/llvm-project/pull/65131 for every backend team as a starter:

/llvm/include/llvm/IR/Intrinsics$backend.td
/llvm/lib/Target/$backend/
/llvm/test/CodeGen/$backend/
/clang/lib/Basic/Targets/$backend*
/clang/lib/Driver/ToolChains/Arch/$backend.*
/clang/lib/CodeGen/Targets/$backend.cpp
/clang/include/clang/Basic/Builtins$backend*

[NoQ]

Clang static analysis groups:

@pr-subscribers-clang-analysis (CFG, flow-sensitive analysis, analysis-based warnings)

clang/include/clang/Analysis/
clang/lib/Analysis/

@pr-subscribers-clang-static-analyzer (the clang static analyzer)

clang/include/clang/StaticAnalyzer/
clang/lib/StaticAnalyzer/
clang/tools/scan-build/
clang/utils/analyzer/
clang/docs/analyzer/

(I think this separation could be better, there’s significant overlap that would probably cause folks to over-subscribe. I’ll open a separate thread to discuss this, maybe we’ll come up with better groups?)

(edit: here’s the thread!)

[snehasish]

Can we have a group for instrumentation and sample profile data:

@pr-subscribers-pgo

llvm/lib/Transforms/Instrumentation/*
llvm/test/Instrumentation/*
compiler-rt/lib/profile/*
compiler-rt/lib/memprof/*
compiler-rt/test/profile/*
compiler-rt/test/memprof/*
llvm/tools/llvm-profdata/*
llvm/tools/llvm-profgen/*
llvm/test/tools/llvm-profdata/*
llvm/test/tools/llvm-profgen/*
llvm/unittests/ProfileData/*

I think that covers all the PGO specific parts we have. Please chime in if there is more to add to this list.
cc @davidxl @WenleiHe

[MaskRay]

@snehasish

llvm/lib/Transforms/Instrumentation/*

Per About code owners - GitHub Docs , you can remove * at the end.

/llvm/lib/Transforms/Instrumentation/ matches *Sanitizer* files, which is probably not desired. This list may work better:

/llvm/lib/Transforms/Instrumentation/CGProfile.cpp
/llvm/lib/Transforms/Instrumentation/ControlHeightReduction.cpp
/llvm/lib/Transforms/Instrumentation/IndirectCallPromotion.cpp
/llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp
/llvm/lib/Transforms/Instrumentation/PGO*
/llvm/lib/Transforms/Instrumentation/ValueProfile*
/llvm/test/Instrumentation/InstrProfiling/
/llvm/test/Transforms/PGOProfile/

[tstellar]

Thanks everyone. I’ve created all the teams and created a pull request to add the file: https://github.com/llvm/llvm-project/pull/65131

Also, I’ve made everyone who requested a team a maintainer of the team, so they can add other people to the teams.

@efriedma-quic

There should be a team for each LLVM backend (llvm/lib/Target/TargetName).

I’m going to hold off on creating teams unless someone explicitly requests it.

@efriedma-quic

Can individual files be added to a team? I’d like, for example, an “clang AArch64” team that covers various bits of clang, but none of those bits are in a distinct directory. Similarly, a “SCEV” team that covers the core bits of ScalarEvolution/ScalarEvolutionExpander.

Yes, you can add individual files.

@philnik

Is it also possible to add blocking reviewers for patches which touch a specific subdirectory? libc++, libc++abi and libunwind currently have blocking reviewer groups, since they are ABI sensitive projects.

The CODEOWNERS file does not support this, but you could write a GitHub Action workflow to do it.

[gargaroff]

I’d love to subscribe to anything GlobalISel related. Could we get pr-subscribers-globalisel?

llvm/docs/GlobalISel
llvm/lib/CodeGen/GlobalISel
llvm/unittests/CodeGen/GlobalISel
llvm/utils/TableGen/GlobalISel
llvm/utils/TableGen/GlobalISelEmitter.cpp
llvm/utils/TableGen/GICombinerEmitter.cpp
llvm/include/llvm/Target/GlobalISel
llvm/include/llvm/CodeGen/GlobalISel

[mehdi_amini]

Seems like llvm/**/*GlobalISel would catch it in a more concise way?

[SixWeining]

Please create pr-subscribers-loongarch with the files as @MaskRay listed here ($backend = LoongArch).

Thank you!

[gargaroff]

Indeed it should Except for the GICombinerEmitter, but I can live without that just fine

[ChuanqiXu]

Also I want to have a team like:

@pr-subscribers-function-specialization

llvm/include/llvm/Transforms/Utils/SCCPSolver.h
llvm/lib/Transforms/Utils/SCCPSolver.cpp
llvm/include/llvm/Transforms/IPO/FunctionSpecialization.h
llvm/lib/Transforms/IPO/FunctionSpecialization.cpp
llvm/test/Transforms/FunctionSpecialization/*

Thanks.

[jh7370]

Sounds good, yeah.

[ChuanqiXu]

@tstellar Also while your effort are appreciated, I am curious about the policy to create new teams in the future. In our plans, do we always need someone like you to create new teams? Or maybe the developers have the rights to do so?

[tobiashieta]

Creating new teams is probably something only an admin could do. I would suggest the future process would involve creating a GitHub Issue with some kind of label that makes it easier to triage.

[smithp35]

I’m going to hold off on creating teams unless someone explicitly requests it.

I’ve left a comment in the PR https://github.com/llvm/llvm-project/pull/65131#discussion_r1311602686, cross-posting here.

We (Arm) would like to follow MaskRays suggested files for a Target given in Input Needed: Teams for Pull Request Subscriptions - #37 by MaskRay
for the ARM and AArch64 backends.

Following the examples set already pr-subscribers-arm and pr-subscribers-aarch64 seem like reasonable names.

[RKSimon]

Please can you add a X86 pr-subscribers-x86 group.

These are the paths I can find of particular interest:

/llvm/include/llvm/IR/IntrinsicsX86.td
/llvm/lib/Target/X86/
/llvm/test/CodeGen/X86/
/llvm/test/MC/X86/
/llvm/test/MC/Disassembler/X86
/llvm/test/Analysis/CostModel/X86/
/llvm/test/tools/llvm-mca/X86/
/clang/lib/Basic/Targets/X86/
/clang/lib/Driver/ToolChains/Arch/X86.*
/clang/lib/CodeGen/Targets/X86.*
/clang/lib/Headers/
/clang/test/CodeGen/X86/
/clang/include/clang/Basic/BuiltinsX86*

[lntue]

Can you please add LLVM libc team: pr-subscribers-libc with the following folders:

libc/**
utils/bazel/llvm-project-overlay/libc/**

Thanks,