Commit Graph

318 Commits

Author SHA1 Message Date
Arthur Eubanks 9bd66b312c [PassManager][Coroutine] Run passes under -O0 conditionally and run GlobalDCE
CoroSplit lowers various coroutine intrinsics. It's a CGSCC pass and
CGSCC passes don't run on unreachable functions. Normally GlobalDCE will
come along and delete unreachable functions, but we don't run GlobalDCE
under -O0, so an unreachable function with coroutine intrinsics may
never have CoroSplit run on it.

This patch adds GlobalDCE when coroutines intrinsics are present. It
also now runs all coroutine passes conditional when coroutine intrinsics
are present. This should also solve the -O0 regression reported in
D105877 due to LazyCallGraph construction.

Fixes https://github.com/llvm/llvm-project/issues/54117

Reviewed By: ChuanqiXu

Differential Revision: https://reviews.llvm.org/D122275
2022-03-23 11:03:26 -07:00
Chuanqi Xu 902f4708fe [NFC] [Coroutines] Remove unnecessary check and constraints on SmallVector
The CoroSplit pass would check the existence of coroutine intrinsic
before starting work. It is not necessary and wasteful since it would
iterate over the Module.

This patch also removes the constraint on the corresponding of the
SmallVector for the possible coroutines in the Modules. The original
value is 4. Given coroutines is used actually in practice. 4 is really
relatively a low threshold.
2022-03-22 14:24:46 +08:00
Fraser Cormack fe74183564 [Coroutines][NFC] Format line to 80 cols 2022-03-17 15:34:24 +00:00
Nikita Popov ce6ca00a92 [CoroSplit] Avoid self-replacement
With opaque pointers, the bitcast might be a no-op, and this can
end up trying to replace a value with itself, which is illegal.
2022-03-14 13:53:31 +01:00
Nikita Popov 479d684ba5 [Coroutines] Support opaque pointers in solveTypeName()
As far as I can tell, these names are only intended to be
informative, so just use a generic "PointerType" for opaque pointers.

The code in solveDIType() also treats pointers as basic types (and
does not try to encode the pointed-to type further), so I believe
this should be fine.

Differential Revision: https://reviews.llvm.org/D121280
2022-03-10 09:33:55 +01:00
Michael Gottesman 0b647fc529 [debug-info] Debug salvage llvm.dbg.addr in original function that point into the coroutine frame when splitting coros.
We are already doing this in the split functions while we clone. This just
handles the original function.

I also updated the coroutine split test to validate that we are always referring
to the msg in the context object instead of in a shadow copy.

rdar://83957028

Reviewed By: aprantl

Differential Revision: https://reviews.llvm.org/D121324
2022-03-09 14:02:09 -08:00
Nikita Popov e81f566de6 [Coroutines] Avoid pointer element access for resume function type
For switch ABI, the function type is always "void (%frame*)", so
just hardcode that rather than fetching it from a pointer element
type.
2022-03-09 14:47:17 +01:00
Nikita Popov 3d9386a349 [CoroFrame] Avoid pointer element type access for swifterror
These must have pointer-to-pointer type, and with opaque pointers
we don't care about the specific pointer type anymore.
2022-03-09 11:15:10 +01:00
Nikita Popov 1bd33691cb [CoroElide] Remove fallback for frame layout determination
Only determine the frame layout based on dereferenceable and align
attributes, and remove the type-based fallback, which is incompatible
with opaque pointers. The dereferenceable attribute is required,
while the align attribute uses default alignment of 1 (commonly,
align 1 attributes do not get placed, relying on default alignment).

The CoroSplit pass producing the resume function adds the necessary
attributes in 7daed35911/llvm/lib/Transforms/Coroutines/CoroSplit.cpp (L840),
and their presence is checked in coro-debug.ll at least.

Differential Revision: https://reviews.llvm.org/D120988
2022-03-07 11:23:02 +01:00
Nikita Popov 9bca4ea364 [Coroutines] Allow FramePtr to be an Argument
With opaque pointers, after splitRetconCoroutine() the FramePtr
may be an Argument rather than an Instruction. With typed pointers,
this currently doesn't happen because the FramePtr would be a
bitcast instruction.

Fix this by making FramePtr a Value and adding a helper for the
"after FramePtr" insertion point, which would be the start of the
function in the Argument case.

Differential Revision: https://reviews.llvm.org/D120994
2022-03-07 10:58:56 +01:00
Nikita Popov 6467d1d275 [CoroFrame] Remove unused insertSpills() return value (NFC) 2022-03-04 15:11:24 +01:00
serge-sans-paille 71c3a5519d Cleanup includes: LLVMAnalysis
Number of lines output by preprocessor:
before: 1065940348
after:  1065307662

Discourse thread: https://discourse.llvm.org/t/include-what-you-use-include-cleanup
Differential Revision: https://reviews.llvm.org/D120659
2022-03-01 18:01:54 +01:00
Michael Gottesman 19279ffc77 [debug-info] If one sees a spill with a dbg.addr use, salvageDebugInfo upon it and don't hoist it.
This ensures that if we have a dbg.addr in a coroutine funclet that is on one of
our function arguments, that the dbg.addr is not mapped to undef and also that
later it isn't hoisted to the front of the basic block. Instead it remains at
its original cloned location.

rdar://83957028

Differential Revision: https://reviews.llvm.org/D119576
2022-02-11 15:15:13 -08:00
Arthur Eubanks 22f4f94256 [CoroFrame][OpaquePtr] Remove getPointerElementType() call
Get it from the byval type instead.
2022-02-11 09:53:20 -08:00
serge-sans-paille ffe8720aa0 Reduce dependencies on llvm/BinaryFormat/Dwarf.h
This header is very large (3M Lines once expended) and was included in location
where dwarf-specific information were not needed.

More specifically, this commit suppresses the dependencies on
llvm/BinaryFormat/Dwarf.h in two headers: llvm/IR/IRBuilder.h and
llvm/IR/DebugInfoMetadata.h. As these headers (esp. the former) are widely used,
this has a decent impact on number of preprocessed lines generated during
compilation of LLVM, as showcased below.

This is achieved by moving some definitions back to the .cpp file, no
performance impact implied[0].

As a consequence of that patch, downstream user may need to manually some extra
files:

llvm/IR/IRBuilder.h no longer includes llvm/BinaryFormat/Dwarf.h
llvm/IR/DebugInfoMetadata.h no longer includes llvm/BinaryFormat/Dwarf.h

In some situations, codes maybe relying on the fact that
llvm/BinaryFormat/Dwarf.h was including llvm/ADT/Triple.h, this hidden
dependency now needs to be explicit.

$ clang++ -E  -Iinclude -I../llvm/include ../llvm/lib/Transforms/Scalar/*.cpp -std=c++14 -fno-rtti -fno-exceptions | wc -l
after:   10978519
before:  11245451

Related Discourse thread: https://llvm.discourse.group/t/include-what-you-use-include-cleanup
[0] https://llvm-compile-time-tracker.com/compare.php?from=fa7145dfbf94cb93b1c3e610582c495cb806569b&to=995d3e326ee1d9489145e20762c65465a9caeab4&stat=instructions

Differential Revision: https://reviews.llvm.org/D118781
2022-02-04 11:44:03 +01:00
serge-sans-paille e188aae406 Cleanup header dependencies in LLVMCore
Based on the output of include-what-you-use.

This is a big chunk of changes. It is very likely to break downstream code
unless they took a lot of care in avoiding hidden ehader dependencies, something
the LLVM codebase doesn't do that well :-/

I've tried to summarize the biggest change below:

- llvm/include/llvm-c/Core.h: no longer includes llvm-c/ErrorHandling.h
- llvm/IR/DIBuilder.h no longer includes llvm/IR/DebugInfo.h
- llvm/IR/IRBuilder.h no longer includes llvm/IR/IntrinsicInst.h
- llvm/IR/LLVMRemarkStreamer.h no longer includes llvm/Support/ToolOutputFile.h
- llvm/IR/LegacyPassManager.h no longer include llvm/Pass.h
- llvm/IR/Type.h no longer includes llvm/ADT/SmallPtrSet.h
- llvm/IR/PassManager.h no longer includes llvm/Pass.h nor llvm/Support/Debug.h

And the usual count of preprocessed lines:
$ clang++ -E  -Iinclude -I../llvm/include ../llvm/lib/IR/*.cpp -std=c++14 -fno-rtti -fno-exceptions | wc -l
before: 6400831
after:  6189948

200k lines less to process is no that bad ;-)

Discourse thread on the topic: https://llvm.discourse.group/t/include-what-you-use-include-cleanup

Differential Revision: https://reviews.llvm.org/D118652
2022-02-02 06:54:20 +01:00
Nikita Popov aa97bc116d [NFC] Remove uses of PointerType::getElementType()
Instead use either Type::getPointerElementType() or
Type::getNonOpaquePointerElementType().

This is part of D117885, in preparation for deprecating the API.
2022-01-25 09:44:52 +01:00
Nikita Popov d29e319263 [OpaquePtrs] Add getNonOpaquePointerElementType() method (NFC)
This method is intended for use in places that cannot be reached
with opaque pointers, or part of deprecated methods. This makes
it easier to see that some uses of getPointerElementType() don't
need further action.

Differential Revision: https://reviews.llvm.org/D117870
2022-01-24 10:03:49 +01:00
Nikita Popov bfbdb5e43e [Coroutines] Avoid some pointer element type accesses
These are just verifying that pointer types are correct, which is
no longer relevant under opaque pointers.
2022-01-21 12:36:19 +01:00
Nikita Popov 9c5b856dac [CoroSplit] Avoid pointer element type accesses
Use isOpaqueOrPointeeTypeMatches() for the assertions instead.
2022-01-21 12:22:09 +01:00
Chuanqi Xu c8ecf12bc3 [Coroutines] Offering llvm.coro.align intrinsic
It is a known problem that we can't align the switch-based coroutine
frame if the alignment exceeds std::max_align_t (which is 16 usually).

We could solve the problem on the middle-end by dynamically transforming
or in the frontend by emitting aligned allocation function.

If we need to solve it in the frontend, the middle end need to offer an
intrinsic to tell the alignment at least. This patch tries to offer such
an intrinsic called llvm.coro.align.

Reviewed By: https://reviews.llvm.org/D117542

Differential revision: https://reviews.llvm.org/D117542
2022-01-19 09:52:45 +08:00
Chuanqi Xu 22225cc5e6 [Coroutines] Handle lifetime markers, bitcast and unused instruciton for symmetric transfer
This fixes bug49888. The root cause for this is that
simplifyTerminatorLeadingToRet didn't handle lifetime markers well.
Another issue also noted in D116327 is that we deleted some inlined
optimization pass in CoroSplit so that simplifyTerminatorLeadingToRet
need to remove dead instructions by hand.

This patch fixes bug49888 by skipping lifetime markers and bitcast
instruction and removing dead instructions by hand in
simplifyTerminatorLeadingToRet.

Reviewed By: junparser

Differential Revision: https://reviews.llvm.org/D116330
2022-01-12 15:58:38 +08:00
Chuanqi Xu 403772ff1c [Coroutines] Enhance symmetric transfer for constant CmpInst
This fixes bug52896.

Simply, some symmetric transfer optimization chances get invalided due
to we delete some inlined optimization passes in 822b92a. This would
cause stack-overflow in some situations which should be avoided by the
design of coroutine. This patch tries to fix this by transforming the
constant CmpInst instruction which was done in the deleted passes.

Reviewed By: rjmccall, junparser

Differential Revision: https://reviews.llvm.org/D116327
2022-01-12 10:14:37 +08:00
Serge Guelton d2cc6c2d0c Use a sorted array instead of a map to store AttrBuilder string attributes
Using and std::map<SmallString, SmallString> for target dependent attributes is
inefficient: it makes its constructor slightly heavier, and involves extra
allocation for each new string attribute. Storing the attribute key/value as
strings implies extra allocation/copy step.

Use a sorted vector instead. Given the low number of attributes generally
involved, this is cheaper, as showcased by

https://llvm-compile-time-tracker.com/compare.php?from=5de322295f4ade692dc4f1823ae4450ad3c48af2&to=05bc480bf641a9e3b466619af43a2d123ee3f71d&stat=instructions

Differential Revision: https://reviews.llvm.org/D116599
2022-01-10 14:49:53 +01:00
Chuanqi Xu e627f4ce0d [NFC] [Coroutines] Rename ReuseFrameSlot to OptimizeFrame
We could use the variable as a flag to indicate if the optimization
is on.
2022-01-05 11:40:27 +08:00
Chuanqi Xu c75cedc237 [Coroutines] Set presplit attribute in Clang and mlir
This fixes bug49264.

Simply, coroutine shouldn't be inlined before CoroSplit. And the marker
for pre-splited coroutine is created in CoroEarly pass, which ran after
AlwaysInliner Pass in O0 pipeline. So that the AlwaysInliner couldn't
detect it shouldn't inline a coroutine. So here is the error.

This patch set the presplit attribute in clang and mlir. So the inliner
would always detect the attribute before splitting.

Reviewed By: rjmccall, ezhulenev

Differential Revision: https://reviews.llvm.org/D115790
2022-01-05 10:25:02 +08:00
Chuanqi Xu 320e4efe99 [C++20] [Coroutines] Mark coroutine done if unhandled_exception throws
According to [dcl.fct.def.coroutine]/p14:
> If the evaluation of the expression promise.unhandled_­exception()
> exits via an exception, the coroutine is considered suspended at the
> final suspend point.

But this is not implemented in clang before. This patch would implement
this feature by marking the coroutine as done at the place of
coro.end(frame, /*InUnwindPath=*/true ).

Reviewed By: rjmccall

Differential Revision: https://reviews.llvm.org/D115219
2021-12-09 14:58:06 +08:00
Chuanqi Xu 352e36e10d [Coroutines] Remove unused coroutine builtin/intrinsics llvm.coro.param (NFC-ish)
I found that the coroutine intrinsic llvm.coro.param in documentation
(https://llvm.org/docs/Coroutines.html#id101) didn't get used actually
since there isn't lowering codes in LLVM. I also checked the
implementation of libstdc++ and libc++. Both of them didn't use
llvm.coro.param. So I am pretty sure that the llvm.coro.param intrinsic
is unused. I think it would be better t to remove it to avoid possible
misleading understandings.

Note: according to [class.copy.elision]/p1.3, this optimization is
allowed by the C++ language specification. Let's make it someday.

Reviewed By: rjmccall

Differential Revision: https://reviews.llvm.org/D115222
2021-12-09 14:40:25 +08:00
Arnold Schwaighofer 7c0e066869 [coro async] Don't use lifetime.start based alloca localization for ABI.Async/ABI.Retcon
Infinite loops can lead to an IR representation where the lifetime.end
intrinsice is missing. The code to do lifetime based optimization then
fails to see that an address escapes (is life) accross a supspend.

Eventually, we could detect such situations and disable it under more narrow
circumstances. For now, do the correct thing.

rdar://83635953

Differential Revision: https://reviews.llvm.org/D110949
2021-12-06 11:50:51 -08:00
Arnold Schwaighofer 64ba9dd943 [coro async] Disable lifetime.start sinking for ABI::Async and ABI::Retcon
It does not handle loops correctly i.e it moves the lifetime.start
intrinsic into a loop rendering the stack object as not alive for part
of the loop.

```
  entry:
    %obj = alloca i8
    lifetime.start(%obj)

    br loop

  loop:
    coro.suspend()
    escape(%obj)
    cond_br %cond, label %exit, label loop

    br loop

  exit:
    lifetime.end(%obj

```

After sinking:

```
  entry:
    %obj = alloca i8
    br loop

  loop:
    coro.suspend()
    lifetime.start(%obj)
    escape(%obj)
    cond_br %cond, label %exit, label loop

    br loop

  exit:
    lifetime.end(%obj

```

rdar://83411917

Differential Revision: https://reviews.llvm.org/D110953
2021-12-06 10:59:43 -08:00
Chuanqi Xu 2ae5011827 [Coroutines] Handle CallBrInst in SalvageDebugInfo
Reviewed by: StephenTozer

Differential Revision: https://reviews.llvm.org/D115139
2021-12-06 22:55:05 +08:00
Chuanqi Xu 84980761a7 [Coroutines] Handle InvokeInst in SalvageDebugInfo
Since coroutine would be splitted into pieces, compiler would move the
dbg.declare intrinsic after the Storage is created to make sure the
corresponding dbg instruction is still available aftet splitted.
However, it would be problematic if the storage instruction is an
InvokeInst, which is a terminator. We couldn't move instruction after an
InvokeInst. This patch tries to move the dbg.declare intrinsic in the
normal destination of the InvokeInst. It should make sense due to the
Storage should be invalid in exception path.
2021-12-03 13:46:54 +08:00
Arnold Schwaighofer 7d11c5dac2 Coro: Remove coro_end and coro_suspend_retcon in private unprocessed functions
We might emit functions that are private and never called. The coro
split pass only processes functions that might be called. Remove
intrinsics that we can't generate code for.

rdar://84619859

Differential Revision: https://reviews.llvm.org/D114021
2021-11-18 07:48:24 -08:00
Kazu Hirata d243cbf8ea [llvm] Use isa instead of dyn_cast (NFC) 2021-11-14 19:40:46 -08:00
Kazu Hirata 0d182d9d1e [Transforms] Use make_early_inc_range (NFC) 2021-11-07 17:03:15 -08:00
Kazu Hirata 1b108ab975 [Transforms] Use make_early_inc_range (NFC) 2021-11-02 18:13:23 -07:00
Kazu Hirata c714da2ceb [Transforms] Use {DenseSet,SetVector,SmallPtrSet}::contains (NFC) 2021-10-31 07:57:32 -07:00
Chuanqi Xu bb16e83932 [NFC] [Coroutines] Use llvm::make_scope_exit to replace self-defined RTTIHelper 2021-10-29 12:14:20 +08:00
Chuanqi Xu ddbf196194 [Coroutines] Ignore partial lifetime markers refer of an alloca
When I playing with Coroutines, I found that it is possible to generate
following IR:
```
%struct = alloca ...
%sub.element = getelementptr %struct, i64 0, i64 index ; index is not
%zero
lifetime.marker.start(%sub.element)
% use of %sub.element
lifetime.marker.end(%sub.element)
store %struct to xxx ;  %struct is escaping!

<suspend points>
```

Then the AllocaUseVisitor would collect the lifetime marker for
sub.element and treat it as the lifetime markers of the alloca! So it
judges that the alloca could be put on the stack instead of the frame by
judging the lifetime markers only.
The root cause for the bug is that AllocaUseVisitor collects wrong
lifetime markers.

This patch fixes this.

Reviewed By: lxfind

Differential Revision: https://reviews.llvm.org/D112216
2021-10-22 09:49:50 +08:00
Kazu Hirata 4f0225f6d2 [Transforms] Migrate from getNumArgOperands to arg_size (NFC)
Note that getNumArgOperands is considered a legacy name.  See
llvm/include/llvm/IR/InstrTypes.h for details.
2021-10-01 09:57:40 -07:00
Arnold Schwaighofer 2df2b27d94 [cora async] Cleanup undefined llvm.coro.async.resume
In situations where the coroutine function is not split we can just
replace the async.resume by null.

rdar://82591919

Differential Revision: https://reviews.llvm.org/D110191
2021-09-30 13:26:53 -07:00
Simon Pilgrim 5f2c53bdf4 Pass some DataLayout arguments by const-ref
Avoid unnecessary copies, reported by MSVC static analyzer.
2021-09-23 15:50:31 +01:00
Kazu Hirata 84b07c9b3a [llvm] Use pop_back_val (NFC) 2021-09-19 13:44:23 -07:00
Kazu Hirata 24c8eaec94 [Transforms] Use make_early_inc_range (NFC) 2021-09-15 19:55:24 -07:00
Arthur Eubanks 6a92ab07cb [NFC][CoroSplit] Directly use Function::getFunctionType() 2021-09-12 21:34:19 -07:00
Kazu Hirata 92c9ff6d5f [IR, Transforms] Use arg_empty (NFC) 2021-09-09 08:50:10 -07:00
Xun Li 2cf30c4769 [Coroutines] Only run verifyFunction in debug mode
verifyFunction can be really slow on large functions. This can significantly slow down compilation in production.
Given that coroutine passes are fairly stable now, we should only run it in debug mode.

Differential Revision: https://reviews.llvm.org/D109198
2021-09-02 17:35:01 -07:00
Adrian Prantl 12de296d84 Tighten heuristic for coroutine debug info workaround.
The OutermostLoad condition is supposed to strip the outermost
DW_OP_deref operation because dbg.declares are implicitly
indirect. This patch makes sure the heuristic is only applied to
dbg.declare intrinsics and only if the outermost instruction is a
load.

This was found while qualifying the latest Swift compiler rebranch.

rdar://82037764
2021-09-01 11:15:36 -07:00
Arthur Eubanks 3f4d00bc3b [NFC] More get/removeAttribute() cleanup 2021-08-17 21:05:41 -07:00
Arthur Eubanks de0ae9e89e [NFC] Cleanup more AttributeList::addAttribute() 2021-08-17 21:05:41 -07:00