Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Coroutines] Support Custom ABIs and plugin libraries #7

Conversation

TylerNowicki
Copy link
Owner

@TylerNowicki TylerNowicki commented Sep 3, 2024

  • Add the llvm.coro.begin.custom intrinsic
  • Add constructors to CoroSplit that take a list of generators that create the custom ABI object.
  • Add has/getCustomABI methods to CoroBeginInst class.
  • Add a unittest for the custom ABI.

See RFC for more info: https://discourse.llvm.org/t/rfc-abi-objects-for-coroutines/81057

@TylerNowicki TylerNowicki force-pushed the users/tylernowicki/coro-refactor7 branch from 6dc5643 to 6a07f3f Compare September 3, 2024 23:41
@TylerNowicki TylerNowicki force-pushed the users/tylernowicki/coro-refactor8 branch from 4a17e4b to dc989dc Compare September 3, 2024 23:51
@TylerNowicki TylerNowicki force-pushed the users/tylernowicki/coro-refactor7 branch from 6a07f3f to f15d042 Compare September 4, 2024 18:17
@TylerNowicki TylerNowicki force-pushed the users/tylernowicki/coro-refactor8 branch from dc989dc to e649f42 Compare September 4, 2024 18:36
TylerNowicki pushed a commit that referenced this pull request Sep 9, 2024
…lvm#107294)

Random testing revealed it's possible to crash the analyzer with the
command line invocation:

clang -cc1 -analyze -analyzer-checker=nullability empty.c

where the source file, empty.c is an empty source file.

```
clang: <root>/clang/lib/StaticAnalyzer/Core/CheckerManager.cpp:56:
   void clang::ento::CheckerManager::finishedCheckerRegistration():
     Assertion `Event.second.HasDispatcher && "No dispatcher registered for an event"' failed.

PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/

Stack dump:
0.      Program arguments: clang -cc1 -analyze -analyzer-checker=nullability nullability-nocrash.c
 #0 ...
 ...
 #7 <addr> clang::ento::CheckerManager::finishedCheckerRegistration()
 #8 <addr> clang::ento::CheckerManager::CheckerManager(clang::ASTContext&,
             clang::AnalyzerOptions&, clang::Preprocessor const&,
             llvm::ArrayRef<std::__cxx11::basic_string<char, std::char_traits<char>,
             std::allocator<char>>>, llvm::ArrayRef<std::function<void (clang::ento::CheckerRegistry&)>>)
```

This commit removes the assertion which failed here, because it was
logically incorrect: it required that if an Event is handled by some
(enabled) checker, then there must be an **enabled** checker which can
emit that kind of Event. It should be OK to disable the event-producing
checkers but enable an event-consuming checker which has different
responsibilities in addition to handling the events.
 
Note that this assertion was in an `#ifndef NDEBUG` block, so this
change does not impact the non-debug builds.

Co-authored-by: Vince Bridgers <[email protected]>
@TylerNowicki TylerNowicki changed the title [llvm/llvm-project][Coroutines] Support Custom ABIs and plugin libraries [Coroutines] Support Custom ABIs and plugin libraries Sep 11, 2024
@TylerNowicki TylerNowicki force-pushed the users/tylernowicki/coro-refactor7 branch 2 times, most recently from 3a12ab8 to 3a9884c Compare September 13, 2024 20:25
@TylerNowicki TylerNowicki force-pushed the users/tylernowicki/coro-refactor8 branch 2 times, most recently from 51fc1ca to 6262ea8 Compare September 13, 2024 20:27
TylerNowicki pushed a commit that referenced this pull request Sep 17, 2024
When SPARC Asan testing is enabled by PR llvm#107405, many Linux/sparc64
tests just hang like
```
#0  0xf7ae8e90 in syscall () from /usr/lib32/libc.so.6
#1  0x701065e8 in __sanitizer::FutexWait(__sanitizer::atomic_uint32_t*, unsigned int) ()
    at compiler-rt/lib/sanitizer_common/sanitizer_linux.cpp:766
#2  0x70107c90 in Wait ()
    at compiler-rt/lib/sanitizer_common/sanitizer_mutex.cpp:35
#3  0x700f7cac in Lock ()
    at compiler-rt/lib/asan/../sanitizer_common/sanitizer_mutex.h:196
#4  Lock ()
    at compiler-rt/lib/asan/../sanitizer_common/sanitizer_thread_registry.h:98
#5  LockThreads ()
    at compiler-rt/lib/asan/asan_thread.cpp:489
#6  0x700e9c8c in __asan::BeforeFork() ()
    at compiler-rt/lib/asan/asan_posix.cpp:157
#7  0xf7ac83f4 in ?? () from /usr/lib32/libc.so.6
Backtrace stopped: previous frame identical to this frame (corrupt stack?)
```
It turns out that this happens in tests using `internal_fork` (e.g.
invoking `llvm-symbolizer`): unlike most other Linux targets, which use
`clone`, Linux/sparc64 has to use `__fork` instead. While `clone`
doesn't trigger `pthread_atfork` handlers, `__fork` obviously does,
causing the hang.

To avoid this, this patch disables `InstallAtForkHandler` and lets the
ASan tests run to completion.

Tested on `sparc64-unknown-linux-gnu`.
@TylerNowicki TylerNowicki force-pushed the users/tylernowicki/coro-refactor7 branch from 3a9884c to c1c55a5 Compare September 19, 2024 21:50
@TylerNowicki TylerNowicki force-pushed the users/tylernowicki/coro-refactor8 branch from 6262ea8 to 0d20cfe Compare September 19, 2024 21:52
@TylerNowicki TylerNowicki force-pushed the users/tylernowicki/coro-refactor7 branch from c1c55a5 to a62f918 Compare October 3, 2024 16:37
@TylerNowicki TylerNowicki force-pushed the users/tylernowicki/coro-refactor8 branch from 0d20cfe to 52688e8 Compare October 3, 2024 16:37
@TylerNowicki TylerNowicki force-pushed the users/tylernowicki/coro-refactor7 branch 2 times, most recently from 9b48ae8 to 33dc7ac Compare October 8, 2024 22:19
vporpo and others added 12 commits October 8, 2024 15:27
Fixed test, needed explicit O_SYMLINK on symbolic link opening.
These comments were confusing because they do not match the code.
In a perfect profile, each positive-execution-count block in the
function’s CFG should be reachable from a positive-execution-count
function entry block through a positive-execution-count path. This new
pass checks how well the BOLT input profile satisfies this “CFG
continuity” property.

More specifically, for each of the hottest 1000 functions, the pass
calculates the function’s fraction of basic block execution counts that
is “unreachable”. It then reports the 95th percentile of the
distribution of the 1000 unreachable fractions in a single BOLT-INFO
line. The smaller the reported value is, the better the BOLT profile
satisfies the CFG continuity property.

The default value of 1000 above can be changed via the hidden BOLT
option `-num-functions-for-continuity-check=[N]`. If more detailed stats
are needed, `-v=1` can be added to the BOLT invocation: the hottest N
functions will be grouped into 5 equally-sized buckets, from the hottest
to the coldest; for each bucket, various summary statistics of the
distribution of the fractions and the raw unreachable execution counts
will be reported.
If ADR instruction references the same function, we can skip relaxation
even if the function is split but ADR is in the main fragment.
This patch implements actual dependencies checking using BatchAA. This
adds memory dep edges between MemDGNodes.
This change will fix the normalization issue with
memref.load when the associated affine map is
reducing the dimension.
This PR fixes llvm#82675

Co-authored-by: Kai Sasaki <[email protected]>
paulwalker-arm and others added 28 commits October 9, 2024 14:17
…oops (llvm#111656)

Properly handles `cycle` branching inside target distribute loops.
Only allow those casts if the bitwidth of the two types match.
When passed -nocudalib/-nogpulib, Cuda's argument handling would bail
out before handling -fcuda-short-ptr, meaning the frontend and backend
data layouts would mismatch.
This bails out if we see an intrinsic with an operand bundle on it, to
make sure we don't process the bundles incorrectly.

Fixes llvm#110382.
…1602)

Many structs in this class have the wrong indentation. To generate this
diff, I touched the first line of each struct and then ran `git
clang-format`. This will make blaming more difficult, but this
autoformatting is difficult to avoid triggering. I think it's best to
push this as one NFC PR.
The LLVM backend has moved from function-wide attributes for making
assurances about potentially unsafe atomic operations (like
"unsafe-fp-atomics") to metadata on individual atomic operations.

This commit adds support for generating this metadata from MLIR.

---------

Co-authored-by: Quinn Dawkins <[email protected]>
When -mno-movt is passed to Clang, the ARM codegen correctly avoids
movt/movw pairs to take the address of __stack_chk_guard in the stack
protector code emitted into the function pro- and epilogues. However,
the Thumb2 codegen fails to do so, and happily emits movw/movt pairs
unless it is generating an ELF binary and the symbol might be in a
different DSO. Let's incorporate a check for useMovt() in the logic
here, so movt/movw are never emitted when -mno-movt is specified.

Suggestions welcome for how/where to add a test case for this.

Signed-off-by: Ard Biesheuvel <[email protected]>
At present, `__builtin_available` is really restrictive with its use.
Overall, this seems like a good thing, since the analyses behind it are
not very expensive.

That said, it's very straightforward to support these two cases:

```
if ((__builtin_available(foo, *))) {
  // ...
}
```

and

```
if (!__builtin_available(foo, *)) {
  // ...
} else {
  // ...
}
```

Seems nice to do so.
This adds the ability to use the GCNRPTrackers during scheduling. These trackers have several advantages over the generic trackers: 1. global live-thru trackers, 2. subregister based RP deltas, and 3. flexible vreg -> PressureSet mappings.

This feature is off-by-default to ease with the roll-out process. In particular, when using the optional trackers, the scheduler will still maintain the generic trackers leading to unnecessary compile time.
Relaxes vector.transfer_write lowering to allow out-of-bound writes.

This aligns lowering with the current hardware specification which does
not update bytes in out-of-bound locations during block stores.
…FromEmptyBlock optimization. (llvm#110715)

In some pathological cases this optimization can spend an unreasonable
amount of time populating the set for predecessors of the successor
block. This change sinks some of that initializing to the point where
it's actually necessary so we can take advantage of the existing
early-exits.

rdar://137063034
Clang-offload-packager allows packaging of images based on an arbitrary
list of key-value pairs where only triple-key is mandatory.

Using target features as a key during packaging is not correct, as clang
does not allow packaging multiple images in one binary which only differ
in a target feature.

TargetID features (xnack and sramecc) anyways are handled using arch-key
and not as target features.
Change-Id: I0b26d5db6d3da8936ab25ee2b1e9002840b9853e
…ion passes after BottomUpVec. (llvm#111223)

The main change is that the main SandboxVectorizer pass no longer has a
pipeline of function passes. Now it is a wrapper that creates sandbox IR
from functions before calling BottomUpVec.

BottomUpVec now builds its own RegionPassManager from the `sbvec-passes`
flag, using a PassRegistry.def file. For now, these region passes are
not run (BottomUpVec doesn't create Regions yet), and only a null pass
for testing exists.

This commit also changes the ownership model for sandboxir::PassManager:
instead of having a PassRegistry that owns passes, and PassManagers that
contain non-owning pointers to the passes, now PassManager owns (via
unique pointers) the passes it contains.

PassRegistry is now deleted, and the logic to parse and create a pass
pipeline is now in PassManager::setPassPipeline.
…#107291)

Trying to codegen these on targets without the instructions should
fail to select. Not sure if all the predicates are correct. We had
a fake one disconnected to a feature which was always true.

Fixes: SWDEV-482274
…e of Region passes after BottomUpVec." (llvm#111727)

Reverts llvm#111223

It broke one of the build bots:

LLVM Buildbot has detected a new failure on builder flang-aarch64-libcxx
running on linaro-flang-aarch64-libcxx while building llvm at step 5
"build-unified-tree".

Full details are available at:
https://lab.llvm.org/buildbot/#/builders/89/builds/8127
Make this emit the same source range as the current interpreter.
…IRGen

Build out the necessary infrastructure for the main entry point into
ClangIR generation -- CIRGenModule. A set of boilerplate classes exist
to facilitate this -- CIRGenerator, CIRGenAction, EmitCIRAction and
CIRGenConsumer. These all mirror the corresponding types from LLVM
generation by Clang's CodeGen.

The main entry point to CIR generation is
`CIRGenModule::buildTopLevelDecl`. It is currently just an empty
function. We've added a test to ensure that the pipeline reaches this
point and doesn't fail, but does nothing else. This will be removed in
one of the subsequent patches that'll add basic `cir.func` emission.

This patch also re-adds `-emit-cir` to the driver. lib/Driver/Driver.cpp
requires that a driver flag exists to facilirate the selection of the
right actions for the driver to create. Without a driver flag you get
the standard behaviors of `-S`, `-c`, etc. If we want to emit CIR IR
and, eventually, bytecode we'll need a driver flag to force this. This
is why `-emit-llvm` is a driver flag. Notably, `-emit-llvm-bc` as a cc1
flag doesn't ever do the right thing. Without a driver flag it is
incorrectly ignored and an executable is emitted. With `-S` a file named
`something.s` is emitted which actually contains bitcode.

Reviewers: AaronBallman, MaskRay, bcardosolopes

Reviewed By: bcardosolopes, AaronBallman

Pull Request: llvm#91007
…lvm#111729)

Use SEND_ERROR (continue processing, but skip generation) instead of
FATAL_ERROR (stop processing and generation). This means that developers
get to see all errors at once, instead of seeing just the first error
and having to reconfigure to discover the next one.
Plugin libraries that use coroutines can do so right now, however, to
provide their own ABI they need to be able to use various headers, some
of which such are required (such as the ABI header). This change exposes
the coro utils and required headers by moving them to
include/llvm/Transforms/Coroutines. My experience with our out-of-tree
plugin ABI has been that at least these headers are needed. The headers
moved are:
* ABI.h (ABI object)
* CoroInstr.h (helpers)
 * Coroshape.h (Shape object)
 * MaterializationUtils.h (helpers)
 * SpillingUtils.h (helpers)
 * SuspendCrossingInfo.h (analysis)

This has no code changes other than those required to move the headers
and these are:
 * include guard name changes
 * include path changes
 * minor clang-format induced changes
 * removal of LLVM_LIBRARY_VISIBILITY
* Add the llvm.coro.begin.custom intrinsic
* Add constructors to CoroSplit that take a list of generators that
  create the custom ABI object.
* Add has/getCustomABI methods to CoroBeginInst class.
* Add a unittest for the custom ABI.
@TylerNowicki TylerNowicki force-pushed the users/tylernowicki/coro-refactor8 branch from 52688e8 to ab72637 Compare October 9, 2024 19:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.