[NFC][DebugInfo] Use iterator-flavour getFirstNonPHI at many call-sites (#123737)

As part of the "RemoveDIs" project, BasicBlock::iterator now carries a
debug-info bit that's needed when getFirstNo

[NFC][DebugInfo] Use iterator-flavour getFirstNonPHI at many call-sites (#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>

show more ...

[IR] Replace of PointerType::getUnqual(Type) with opaque version (NFC) (#123909)

Follow up to https://github.com/llvm/llvm-project/issues/123569

[WebAssembly] Use heterogenous lookups with std::set (NFC) (#114930)

[WebAssembly] Avoid repeated hash lookups (NFC) (#112124)

[NFC] Rename `Intrinsic::getDeclaration` to `getOrInsertDeclaration` (#111752)

Rename the function to reflect its correct behavior and to be consistent
with `Module::getOrInsertFunction`. This is a

[NFC] Rename `Intrinsic::getDeclaration` to `getOrInsertDeclaration` (#111752)

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).

show more ...

[LLVM] Use {} instead of std::nullopt to initialize empty ArrayRef (#109133)

It is almost always simpler to use {} instead of std::nullopt to
initialize an empty ArrayRef. This patch changes all oc

[LLVM] Use {} instead of std::nullopt to initialize empty ArrayRef (#109133)

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.

show more ...

Reapply "[WebAssembly] Fix phi handling for Wasm SjLj (#99730)"

This reapplies #99730. #99730 contained a nondeterministic iteration
which failed the reverse-iteration bot
(https://lab.llvm.org/buil

Reapply "[WebAssembly] Fix phi handling for Wasm SjLj (#99730)"

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
```

show more ...

Revert "[WebAssembly] Fix phi handling for Wasm SjLj (#99730)"

This reverts commit 2bf71b8bc851b49745b795f228037db159005570.
This broke the builbot at
https://lab.llvm.org/buildbot/#/builders/110/bu

Revert "[WebAssembly] Fix phi handling for Wasm SjLj (#99730)"

This reverts commit 2bf71b8bc851b49745b795f228037db159005570.
This broke the builbot at
https://lab.llvm.org/buildbot/#/builders/110/builds/474.

show more ...

[WebAssembly] Fix phi handling for Wasm SjLj (#99730)

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
`c

[WebAssembly] Fix phi handling for Wasm SjLj (#99730)

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.

show more ...

[IRBuilder] Don't include Module.h (NFC) (#97159)

This used to be necessary to fetch the DataLayout, but isn't anymore.

Revert "[IR][NFC] Update IRBuilder to use InsertPosition (#96497)"

Reverts the above commit, as it updates a common header function and
did not update all callsites:

https://lab.llvm.org/buildbot

Revert "[IR][NFC] Update IRBuilder to use InsertPosition (#96497)"

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.

show more ...

[IR][NFC] Update IRBuilder to use InsertPosition (#96497)

Uses the new InsertPosition class (added in #94226) to simplify some of
the IRBuilder interface, and removes the need to pass a BasicBlock

[IR][NFC] Update IRBuilder to use InsertPosition (#96497)

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.

show more ...

Revert "[WebAssembly] Remove threwValue comparison after __wasm_setjmp_test (#86633)"

This reverts commit 52431fdb1ab8d29be078edd55250e06381e4b6b0.

The PR assumed `__threwValue` couldn't be 0, but

Revert "[WebAssembly] Remove threwValue comparison after __wasm_setjmp_test (#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.

show more ...

[WebAssembly] Remove threwValue comparison after __wasm_setjmp_test (#86633)

Currently the code thinks a `longjmp` occurred if both `__THREW__` and
`__threwValue` are nonzero. But `__threwValue` ca

[WebAssembly] Remove threwValue comparison after __wasm_setjmp_test (#86633)

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

show more ...

[WebAssembly] Implement an alternative translation for -wasm-enable-sjlj (#84137)

Instead of maintaining per-function-invocation malloc()'ed tables to track which functions each label belongs to, st

[WebAssembly] Implement an alternative translation for -wasm-enable-sjlj (#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

show more ...

[NFC][RemoveDIs] Use iterators for insertion at various call-sites (#84736)

These are the last remaining "trivial" changes to passes that use
Instruction pointers for insertion. All of this should

[NFC][RemoveDIs] Use iterators for insertion at various call-sites (#84736)

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>

show more ...

[llvm] Use StringRef::{starts,ends}_with (NFC) (#74956)

This patch replaces uses of StringRef::{starts,ends}with with
StringRef::{starts,ends}_with for consistency with
std::{string,string_view}::

[llvm] Use StringRef::{starts,ends}_with (NFC) (#74956)

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.

show more ...

Move CallInst::CreateFree to IRBuilderBase

Similarly to D158861 I'm moving the `CreateFree` method from `CallInst` to `IRBuilderBase`.

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

[llvm] Move CallInst::CreateMalloc to IRBuilderBase::CreateMalloc

This removes `CreateMalloc` from `CallInst` and adds it to the `IRBuilderBase`
class.

We no longer needed the `Instruction *InsertB

[llvm] Move CallInst::CreateMalloc to IRBuilderBase::CreateMalloc

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

show more ...

[NFC][RemoveDIs] Use iterators over inst-pointers when using IRBuilder

This patch adds a two-argument SetInsertPoint method to IRBuilder that
takes a block/iterator instead of an instruction, and up

[NFC][RemoveDIs] Use iterators over inst-pointers when using IRBuilder

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

show more ...

[llvm] Remove SmallSet from MachineInstr.h

`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

[llvm] Remove SmallSet from MachineInstr.h

`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

show more ...

Stop using legacy helpers indicating typed pointer types. NFC

Since we no longer support typed LLVM IR pointer types, the code can
be simplified into for example using PointerType::get directly inst

Stop using legacy helpers indicating typed pointer types. NFC

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

show more ...

[llvm] Replace uses of Type::getPointerTo (NFC)

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 unne

[llvm] Replace uses of Type::getPointerTo (NFC)

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

show more ...

[WebAssembly] Replace Bugzilla links with Github issues

Reviewed By: dschuff, asb

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

[Target] Use *{Set,Map}::contains (NFC)