| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
After #149310 lifetime intrinsics require an alloca argument, an
invariant that this pass can break.
I've fixed this in two ways:
* First, move static allocas into the entry block. Currently, the way
the pass splits the entry block makes all allocas dynamic, which I
assume was not actually intended. This will avoid unnecessary SSA
reconstruction for allocas as well, and thus avoid the problem.
* If this fails (for dynamic allocas) drop all lifetime intrinsics if
any one of them would require a rewrite during SSA reconstruction.
Fixes https://github.com/llvm/llvm-project/issues/150498.
|
|
These are identified by misc-include-cleaner. I've filtered out those
that break builds. Also, I'm staying away from llvm-config.h,
config.h, and Compiler.h, which likely cause platform- or
compiler-specific build failures.
|
|
|
|
|
|
Use `poison` instead of `undef` as a placeholder for phi entries of
unreachable predecessors.
|
|
Unlike in Itanium EH IR, WinEH IR's unwinding instructions (e.g.
`invoke`s) can have multiple possible unwind destinations.
For example:
```ll
entry:
invoke void @foo()
to label %cont unwind label %catch.dispatch
catch.dispatch: ; preds = %entry
%0 = catchswitch within none [label %catch.start] unwind label %terminate
catch.start: ; preds = %catch.dispatch
%1 = catchpad within %0 [ptr null]
...
terminate: ; preds = %catch.dispatch
%2 = catchpad within none []
...
...
```
In this case, if an exception is not caught by `catch.dispatch` (and
thus `catch.start`), it should next unwind to `terminate`.
`findUnwindDestination` in ISel gathers the list of this unwind
destinations traversing the unwind edges:
https://github.com/llvm/llvm-project/blob/ae42f071032b29821beef6a33771258086bbbb1c/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp#L2089-L2150
But we don't use that, and instead use our custom
`findWasmUnwindDestinations` that only adds the first unwind
destination, `catch.start`, to the successor list of `entry`, and not
`terminate`:
https://github.com/llvm/llvm-project/blob/ae42f071032b29821beef6a33771258086bbbb1c/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp#L2037-L2087
The reason behind it was, as described in the comment block in the code,
it was assumed that there always would be an `invoke` that connects
`catch.start` and `terminate`. In case of `catch (type)`, there will be
`call void @llvm.wasm.rethrow()` in `catch.start`'s predecessor that
unwinds to the next destination. For example:
https://github.com/llvm/llvm-project/blob/0db702ac8e06911478615ac537f75ac778817c04/llvm/test/CodeGen/WebAssembly/exception.ll#L429-L430
In case of `catch (...)`, `__cxa_end_catch` can throw, so it becomes an
`invoke` that unwinds to the next destination. For example:
https://github.com/llvm/llvm-project/blob/0db702ac8e06911478615ac537f75ac778817c04/llvm/test/CodeGen/WebAssembly/exception.ll#L537-L538
So the unwind ordering relationship between `catch.start` and
`terminate` here would be preserved.
But turns out this assumption does not always hold. For example:
```ll
entry:
invoke void @foo()
to label %cont unwind label %catch.dispatch
catch.dispatch: ; preds = %entry
%0 = catchswitch within none [label %catch.start] unwind label %terminate
catch.start: ; preds = %catch.dispatch
%1 = catchpad within %0 [ptr null]
...
call void @_ZSt9terminatev()
unreachable
terminate: ; preds = %catch.dispatch
%2 = catchpad within none []
call void @_ZSt9terminatev()
unreachable
...
```
In this case there is no `invoke` that connects `catch.start` to
`terminate`. So after `catch.dispatch` BB is removed in ISel,
`terminate` is considered unreachable and incorrectly removed in DCE.
This makes Wasm just use the general `findUnwindDestination`. In that
case `entry`'s successor is going to be [`catch.start`, `terminate`]. We
can get the first unwind destination by just traversing the list from
the front.
---
This required another change in WinEHPrepare. WinEHPrepare demotes all
PHIs in EH pads because they are funclets in Windows and funclets can't
have PHIs. When used in Wasm they are not funclets so we don't need to
do that wholesale but we still need to demote PHIs in `catchswitch` BBs
because they are deleted during ISel. (So we created
[`-demote-catchswitch-only`](https://github.com/llvm/llvm-project/blob/a5588b6d20590a10db0f1a2046fba4d9f205ed68/llvm/lib/CodeGen/WinEHPrepare.cpp#L57-L59)
option for that)
But turns out we need to remove PHIs that have a `catchswitch` BB as an
incoming block too:
```ll
...
catch.dispatch:
%0 = catchswitch within none [label %catch.start] unwind label %terminate
catch.start:
...
somebb:
...
ehcleanup ; preds = %catch.dispatch, %somebb
%1 = phi i32 [ 10, %catch.dispatch ], [ 20, %somebb ]
...
```
In this case the `phi` in `ehcleanup` BB should be demoted too because
`catch.dispatch` BB will be removed in ISel so one if its incoming block
will be gone. This pattern didn't manifest before presumably due to how
`findWasmUnwindDestinations` worked. (In this example, in our
`findWasmUnwindDestinations`, `catch.dispatch` would have had only one
successor, `catch.start`. But now `catch.dispatch` has both
`catch.start` and `ehcleanup` as successors, revealing this bug. This
case is
[represented](https://github.com/llvm/llvm-project/blob/ab87206c4b95aa0b5047facffb5f78f7fe6ac269/llvm/test/CodeGen/WebAssembly/exception.ll#L445)
by `rethrow_terminator` function in `exception.ll` (or
`exception-legacy.ll`) and without the WinEHPrepare fix it will crash.
---
Discovered by the reproducer provided in #126916, even though the bug
reported there was not this one.
|
|
|
|
(#128564)
This change effectively reverts 296ccef
(https://reviews.llvm.org/D77192)
Most of these symbols are just normal C symbols that get imported from
wither libcompiler-rt or from emscripten's JS library code. In most
cases it should not be necessary to give them explicit import names.
The advantage of doing this is that we can wasm-ld can/will fail with a
useful error message when these symbols are missing. As opposed to today
where it will simply import them and defer errors until later (when they
are less specific).
|
|
(#123737)
As part of the "RemoveDIs" project, BasicBlock::iterator now carries a
debug-info bit that's needed when getFirstNonPHI and similar feed into
instruction insertion positions. Call-sites where that's necessary were
updated a year ago; but to ensure some type safety however, we'd like to
have all calls to getFirstNonPHI use the iterator-returning version.
This patch changes a bunch of call-sites calling getFirstNonPHI to use
getFirstNonPHIIt, which returns an iterator. All these call sites are
where it's obviously safe to fetch the iterator then dereference it. A
follow-up patch will contain less-obviously-safe changes.
We'll eventually deprecate and remove the instruction-pointer
getFirstNonPHI, but not before adding concise documentation of what
considerations are needed (very few).
---------
Co-authored-by: Stephen Tozer <Melamoto@gmail.com>
|
|
Follow up to https://github.com/llvm/llvm-project/issues/123569
|
|
|
|
|
|
Rename the function to reflect its correct behavior and to be consistent
with `Module::getOrInsertFunction`. This is also in preparation of
adding a new `Intrinsic::getDeclaration` that will have behavior similar
to `Module::getFunction` (i.e, just lookup, no creation).
|
|
It is almost always simpler to use {} instead of std::nullopt to
initialize an empty ArrayRef. This patch changes all occurrences I could
find in LLVM itself. In future the ArrayRef(std::nullopt_t) constructor
could be deprecated or removed.
|
|
This reapplies #99730. #99730 contained a nondeterministic iteration
which failed the reverse-iteration bot
(https://lab.llvm.org/buildbot/#/builders/110/builds/474) and reverted
in
https://github.com/llvm/llvm-project/commit/f3f0d9928f982cfd302351f418bcc5b63cc1bb9d.
The fix is make the order of iteration of new predecessors
determintistic by using `SmallSetVector`.
```diff
--- a/llvm/lib/Target/WebAssembly/WebAssemblyLowerEmscriptenEHSjLj.cpp
+++ b/llvm/lib/Target/WebAssembly/WebAssemblyLowerEmscriptenEHSjLj.cpp
@@ -1689,7 +1689,7 @@ void WebAssemblyLowerEmscriptenEHSjLj::handleLongjmpableCallsForWasmSjLj(
}
}
- SmallDenseMap<BasicBlock *, SmallPtrSet<BasicBlock *, 4>, 4>
+ SmallDenseMap<BasicBlock *, SmallSetVector<BasicBlock *, 4>, 4>
UnwindDestToNewPreds;
for (auto *CI : LongjmpableCalls) {
// Even if the callee function has attribute 'nounwind', which is true for
```
|
|
This reverts commit 2bf71b8bc851b49745b795f228037db159005570.
This broke the builbot at
https://lab.llvm.org/buildbot/#/builders/110/builds/474.
|
|
In Wasm SjLj, longjmpable `call`s that in functions that call `setjmp`
are converted into `invoke`s. Those `invoke`s are meant to unwind to
`catch.dispatch.longjmp` to figure out which `setjmp` those `longjmp`
buffers belong to:
https://github.com/llvm/llvm-project/blob/fada9227325b3eaa0bdc09a486f29a7f08b7b3fb/llvm/lib/Target/WebAssembly/WebAssemblyLowerEmscriptenEHSjLj.cpp#L250-L260
But in case a longjmpable call is within another `catchpad` or
`cleanuppad` scope, to maintain the nested scope structure, we should
make them unwind to the scope's next unwind destination and not directly
to `catch.dispatch.longjmp`:
https://github.com/llvm/llvm-project/blob/fada9227325b3eaa0bdc09a486f29a7f08b7b3fb/llvm/lib/Target/WebAssembly/WebAssemblyLowerEmscriptenEHSjLj.cpp#L1698-L1727
In this case the longjmps will eventually unwind to
`catch.dispatch.longjmp` and be handled there.
In this case, it is possible that the unwind destination (which is an
existing `catchpad` or `cleanuppad`) may already have `phi`s. And
because the unwind destinations get new predecessors because of the
newly created `invoke`s, those `phi`s need to have new entries for those
new predecessors.
This adds new preds as new incoming blocks to those `phi`s, and we use a
separate `SSAUpdater` to calculate the correct incoming values to those
blocks.
I have assumed `SSAUpdaterBulk` used in `rebuildSSA` would take care of
these things, but apparently it doesn't. It takes available defs and
adds `phi`s in the defs' dominance frontiers, i.e., where each def's
dominance ends, and rewrites other uses based on the newly added `phi`s.
But it doesn't add entries to existing `phi`s, and the case in this bug
may not even involve dominance frontiers; this bug is simply about
existing `phis`s that have gained new preds need new entries for them.
It is kind of surprising that this bug was only reported recently, given
that this pass has not been changed much in years.
Fixes #97496 and fixes
https://github.com/emscripten-core/emscripten/issues/22170.
|
|
This used to be necessary to fetch the DataLayout, but isn't anymore.
|
|
Reverts the above commit, as it updates a common header function and
did not update all callsites:
https://lab.llvm.org/buildbot/#/builders/29/builds/382
This reverts commit 6481dc57612671ebe77fe9c34214fba94e1b3b27.
|
|
Uses the new InsertPosition class (added in #94226) to simplify some of
the IRBuilder interface, and removes the need to pass a BasicBlock
alongside a BasicBlock::iterator, using the fact that we can now get the
parent basic block from the iterator even if it points to the sentinel.
This patch removes the BasicBlock argument from each constructor or call
to setInsertPoint.
This has no functional effect, but later on as we look to remove the
`Instruction *InsertBefore` argument from instruction-creation
(discussed
[here](https://discourse.llvm.org/t/psa-instruction-constructors-changing-to-iterator-only-insertion/77845)),
this will simplify the process by allowing us to deprecate the
InsertPosition constructor directly and catch all the cases where we use
instructions rather than iterators.
|
|
(#86633)"
This reverts commit 52431fdb1ab8d29be078edd55250e06381e4b6b0.
The PR assumed `__threwValue` couldn't be 0, but it could be when the
thrown thing is not a longjmp but an exception, so that `if` check was
actually necessary.
|
|
Currently the code thinks a `longjmp` occurred if both `__THREW__` and
`__threwValue` are nonzero. But `__threwValue` can be 0, and the
`longjmp` library function should change it to 1 in case it is 0:
https://en.cppreference.com/w/c/program/longjmp
Emscripten libraries were not consistent about that, but after
https://github.com/emscripten-core/emscripten/pull/21493 and
https://github.com/emscripten-core/emscripten/pull/21502, we correctly
pass 1 in case the input is 0. So there will be no case `__threwValue`
is 0. And regardless of what `longjmp` library function does, treating
`longjmp`'s 0 input to its second argument as "not longjmping" doesn't
seem right.
I'm not sure where that `__threwValue` checking came from, but probably
I was porting then fastcomp's implementation and moved this part just
verbatim:
https://github.com/emscripten-core/emscripten-fastcomp/blob/9bdc7bb4fc595fe05a021b06fe350e8494a741a1/lib/Target/JSBackend/CallHandlers.h#L274-L278
Just for the context, how this was discovered:
https://github.com/emscripten-core/emscripten/pull/21502#pullrequestreview-1942160300
|
|
(#84137)
Instead of maintaining per-function-invocation malloc()'ed tables to track which functions each label belongs to, store the equivalent info in jump buffers (jmp_buf) themselves.
Also, use a less emscripten-looking ABI symbols:
```
saveSetjmp -> __wasm_setjmp
testSetjmp -> __wasm_setjmp_test
getTempRet0 -> (removed)
__wasm_longjmp -> (no change)
```
While I want to use this for WASI, it should work for emscripten as well.
An example runtime and a few tests:
https://github.com/yamt/garbage/tree/wasm-sjlj-alt2/wasm/longjmp
wasi-libc version of the runtime:
https://github.com/WebAssembly/wasi-libc/pull/483
emscripten version of the runtime:
https://github.com/emscripten-core/emscripten/pull/21502
Discussion:
https://docs.google.com/document/d/1ZvTPT36K5jjiedF8MCXbEmYjULJjI723aOAks1IdLLg/edit
|
|
These are the last remaining "trivial" changes to passes that use
Instruction pointers for insertion. All of this should be NFC, it's just
changing the spelling of how we identify a position.
In one or two locations, I'm also switching uses of getNextNode etc to
using std::next with iterators. This too should be NFC.
---------
Merged by: Stephen Tozer <stephen.tozer@sony.com>
|
|
This patch replaces uses of StringRef::{starts,ends}with with
StringRef::{starts,ends}_with for consistency with
std::{string,string_view}::{starts,ends}_with in C++20.
I'm planning to deprecate and eventually remove
StringRef::{starts,ends}with.
|
|
Similarly to D158861 I'm moving the `CreateFree` method from `CallInst` to `IRBuilderBase`.
Differential Revision: https://reviews.llvm.org/D159418
|
|
This removes `CreateMalloc` from `CallInst` and adds it to the `IRBuilderBase`
class.
We no longer needed the `Instruction *InsertBefore` and
`BasicBlock *InsertAtEnd` arguments of the `createMalloc` helper
function because we're using `IRBuilder` now. That's why I we also don't
need 4 `CreateMalloc` functions, but only two.
Differential Revision: https://reviews.llvm.org/D158861
|
|
This patch adds a two-argument SetInsertPoint method to IRBuilder that
takes a block/iterator instead of an instruction, and updates many call
sites to use it. The motivating reason for doing this is given here [0],
we'd like to pass around more information about the position of debug-info
in the iterator object. That necessitates passing iterators around most of
the time.
[0] https://discourse.llvm.org/t/rfc-instruction-api-changes-needed-to-eliminate-debug-intrinsics-from-ir/68939
Differential Revision: https://reviews.llvm.org/D152468
|
|
`MachineInstr.h` is a commonly included file and this includes
`llvm/ADT/SmallSet.h` for one function `getUsedDebugRegs()`, which is
used only in one place.
According to `ClangBuildAnalyzer` (run solely on building LLVM, no other
projects) the second most expensive template to instantiate is the
`SmallSet::insert` method used in the `inline` implementation in
`getUsedDebugRegs()`:
```
**** Templates that took longest to instantiate:
554239 ms: std::unordered_map<int, int> (2826 times, avg 196 ms)
521187 ms: llvm::SmallSet<llvm::Register, 4>::insert (930 times, avg 560
ms)
...
```
By removing this method and putting its implementation in the one call
site we greatly reduce the template instantiation time and reduce the
number of includes.
When copying the implementation, I removed a check on `MO.getReg()` as
this is checked within `MO.isVirtual()`.
Differential Revision: https://reviews.llvm.org/D157720
|
|
Since we no longer support typed LLVM IR pointer types, the code can
be simplified into for example using PointerType::get directly instead
of using Type::getInt8PtrTy and Type::getInt32PtrTy etc.
Differential Revision: https://reviews.llvm.org/D156733
|
|
Partial progress towards removing in-tree uses of `Type::getPointerTo`,
before we can deprecate the API.
If the API is used solely to support an unnecessary bitcast, get rid of
the bitcast as well.
Reviewed By: nikic
Differential Revision: https://reviews.llvm.org/D153933
|
|
Reviewed By: dschuff, asb
Differential Revision: https://reviews.llvm.org/D145966
|
|
|
|
WebAssemblyUtils depends on CodeGen which depends on all middle end
optimization libraries.
This component is used by WebAssembly's AsmParser, Disassembler, and
MCTargetDesc libraries. Because of this, any MC layer tool built with
WebAssembly support includes a larger portion of LLVM than it should.
To fix this I've created an MC only version of WebAssemblyTypeUtilities.cpp
in MCTargetDesc to be used by the MC components.
This shrinks llvm-objdump and llvm-mc on my local release+asserts
build by 5-6 MB.
Reviewed By: MaskRay, aheejin
Differential Revision: https://reviews.llvm.org/D144354
|
|
value() has undesired exception checking semantics and calls
__throw_bad_optional_access in libc++. Moreover, the API is unavailable without
_LIBCPP_NO_EXCEPTIONS on older Mach-O platforms (see
_LIBCPP_AVAILABILITY_BAD_OPTIONAL_ACCESS).
This fixes clang.
|
|
This is part of a series of patches that aim at making Function::getInstList() private.
Differential Revision: https://reviews.llvm.org/D139971
|
|
This patch mechanically replaces None with std::nullopt where the
compiler would warn if None were deprecated. The intent is to reduce
the amount of manual work required in migrating from Optional to
std::optional.
This is part of an effort to migrate from llvm::Optional to
std::optional:
https://discourse.llvm.org/t/deprecating-llvm-optional-x-hasvalue-getvalue-getvalueor/63716
|
|
As suggested on D135572, return Optional<> from getAllocSizeArgs()
rather than the peculiar pair(0, 0) sentinel.
The method on Attribute itself does not return Optional, because
the attribute must exist in that case.
|
|
wasm64 mode
Differential Revision: https://reviews.llvm.org/D132091
|
|
|
|
|
|
This patch replaces Optional::hasValue with the implicit cast to bool
in conditionals only.
|
|
This reverts commit aa8feeefd3ac6c78ee8f67bf033976fc7d68bc6d.
|
|
|
|
This includes a function name and a relevant instruction in error
messages when possible, making them more helpful.
Reviewed By: dschuff
Differential Revision: https://reviews.llvm.org/D120678
|
|
This makes three thread local variables (`__THREW__`, `__threwValue`,
and `__wasm_lpad_context`) unconditionally thread local. If the target
doesn't support TLS, they will be downgraded to normal variables in
`stripThreadLocals`. This makes the object not linkable with other
objects using shared memory, which is what we intend here; these
variables should be thread local when used with shared memory. This is
what we initially tried in D88262.
But D88323 changed this: It only created these variables when threads
were supported, because `__THREW__` and `__threwValue` were always
generated even if Emscripten EH/SjLj was not used, making all objects
built without threads not linkable with shared memory, which was too
restrictive. But sometimes this is not safe. If we build an object using
variables such as `__THREW__` without threads, it can be linked to other
objects using shared memory, because the original object's `__THREW__`
was not created thread local to begin with.
So this CL basically reverts D88323 with some additional improvements:
- This checks each of the functions and global variables created within
`LowerEmscriptenEHSjLj` pass and removes it if it's not used at the
end of the pass. So only modules using those variables will be
affected.
- Moves `CoalesceFeaturesAndStripAtomics` and `AtomicExpand` passes
after all other IR pasess that can create thread local variables. It
is not sufficient to move them to the end of `addIRPasses`, because
`__wasm_lpad_context` is created in `WasmEHPrepare`, which runs inside
`addPassesToHandleExceptions`, which runs before `addISelPrepare`. So
we override `addISelPrepare` and move atomic/TLS stripping and
expanding passes there.
This also removes merges `TLS` and `NO-TLS` FileCheck lines into one
`CHECK` line, because in the bitcode level we always create them as
thread local. Also some function declarations are deleted `CHECK` lines
because they are unused.
Reviewed By: tlively, sbc100
Differential Revision: https://reviews.llvm.org/D120013
|
|
These global TLS symbols are shared across all shared libraries and
therefor should not be assumed to be local to the current module.
Also add new error in the linker when TLS relocations are used against
undefined symbols. TLS relocations are offsets into the current modules
tls data segment, and don't make sense for undefined symbols which are
modeled as global imports.
Fixes: https://github.com/emscripten-core/emscripten/issues/13398
Differential Revision: https://reviews.llvm.org/D119630
|
|
The pointers are dereferenced immediately, so assert the cast is correct instead of returning nullptr
|
|
This is similar to D116619, but now it handles `invoke`s. The reason we
didn't handle `invoke`s back then was we didn't support Wasm EH + Wasm
SjLj together, and the only case SjLj transformation will see `invoke`s
is when we are using Wasm EH. (In Emscripten EH, they would have been
transformed to `call`s to invoke wrappers.)
But after D117610 we support Wasm EH + Wasm SjLj together and we can
nullify `invoke`s to `setjmp` when there is no other longjmpable calls
within the function. Actually this is very unlikely to happen in
practice, because we treat destructors as longjmpable and also treat
`__cxa_end_catch` as longjmpable even if it is not.
Reviewed By: dschuff
Differential Revision: https://reviews.llvm.org/D118408
|
|
Wasm SjLj converts longjmpable calls into `invoke`s that unwind to
`%catch.longjmp.dispatch` BB, from where we check if the thrown
exception is a `longjmp`. But in case a call already has a `funclet`
attribute, i.e., it is within a catch scope, we have to unwind to its
unwind destination first to preserve the scoping structure. That will
eventually unwind to `%catch.longjmp.dispatch`, because all
`catchswitch` and `cleanupret` that unwind to caller are redirected to
`%catch.dispatch.longjmp` during Wasm SjLj transformation.
But the prevous code assumed `cleanuppad`'s parent pad was always an
instruction, and didn't handle when a `cleanuppad`'s parent is `none`.
This CL handles this case, and makes the `while` loop more intuitive by
removing `FromPad` condition and explicitly inserting `break`s.
Reviewed By: dschuff
Differential Revision: https://reviews.llvm.org/D118407
|
