- Mar 05, 2021
-
-
Heejin Ahn authored
Background: Wasm EH, while using Windows EH (catchpad/cleanuppad based) IR, uses Itanium-based libraries and ABIs with some modifications. `__clang_call_terminate` is a wrapper generated in Clang's Itanium C++ ABI implementation. It contains this code, in C-style pseudocode: ``` void __clang_call_terminate(void *exn) { __cxa_begin_catch(exn); std::terminate(); } ``` So this function is a wrapper to call `__cxa_begin_catch` on the exception pointer before termination. In Itanium ABI, this function is called when another exception is thrown while processing an exception. The pointer for this second, violating exception is passed as the argument of this `__clang_call_terminate`, which calls `__cxa_begin_catch` with that pointer and calls `std::terminate` to terminate the program. The spec (https://libcxxabi.llvm.org/spec.html) for `__cxa_begin_catch` says, ``` When the personality routine encounters a termination condition, it will call __cxa_begin_catch() to mark the exception as handled and then call terminate(), which shall not return to its caller. ``` In wasm EH's Clang implementation, this function is called from cleanuppads that terminates the program, which we also call terminate pads. Cleanuppads normally don't access the thrown exception and the wasm backend converts them to `catch_all` blocks. But because we need the exception pointer in this cleanuppad, we generate `wasm.get.exception` intrinsic (which will eventually be lowered to `catch` instruction) as we do in the catchpads. But because terminate pads are cleanup pads and should run even when a foreign exception is thrown, so what we have been doing is: 1. In `WebAssemblyLateEHPrepare::ensureSingleBBTermPads()`, we make sure terminate pads are in this simple shape: ``` %exn = catch call @__clang_call_terminate(%exn) unreachable ``` 2. In `WebAssemblyHandleEHTerminatePads` pass at the end of the pipeline, we attach a `catch_all` to terminate pads, so they will be in this form: ``` %exn = catch call @__clang_call_terminate(%exn) unreachable catch_all call @std::terminate() unreachable ``` In `catch_all` part, we don't have the exception pointer, so we call `std::terminate()` directly. The reason we ran HandleEHTerminatePads at the end of the pipeline, separate from LateEHPrepare, was it was convenient to assume there was only a single `catch` part per `try` during CFGSort and CFGStackify. --- Problem: While it thinks terminate pads could have been possibly split or calls to `__clang_call_terminate` could have been duplicated, `WebAssemblyLateEHPrepare::ensureSingleBBTermPads()` assumes terminate pads contain no more than calls to `__clang_call_terminate` and `unreachable` instruction. I assumed that because in LLVM very limited forms of transformations are done to catchpads and cleanuppads to maintain the scoping structure. But it turned out to be incorrect; passes can merge cleanuppads into one, including terminate pads, as long as the new code has a correct scoping structure. One pass that does this I observed was `SimplifyCFG`, but there can be more. After this transformation, a single cleanuppad can contain any number of other instructions with the call to `__clang_call_terminate` and can span many BBs. It wouldn't be practical to duplicate all these BBs within the cleanuppad to generate the equivalent `catch_all` blocks, only with calls to `__clang_call_terminate` replaced by calls to `std::terminate`. Unless we do more complicated transformation to split those calls to `__clang_call_terminate` into a separate cleanuppad, it is tricky to solve. --- Solution (?): This CL just disables the generation and use of `__clang_call_terminate` and calls `std::terminate()` directly in its place. The possible downside of this approach can be, because the Itanium ABI intended to "mark" the violating exception handled, we don't do that anymore. What `__cxa_begin_catch` actually does is increment the exception's handler count and decrement the uncaught exception count, which in my opinion do not matter much given that we are about to terminate the program anyway. Also it does not affect info like stack traces that can be possibly shown to developers. And while we use a variant of Itanium EH ABI, we can make some deviations if we choose to; we are already different in that in the current version of the EH spec we don't support two-phase unwinding. We can possibly consider a more complicated transformation later to reenable this, but I don't think that has high priority. Changes in this CL contains: - In Clang, we don't generate a call to `wasm.get.exception()` intrinsic and `__clang_call_terminate` function in terminate pads anymore; we simply generate calls to `std::terminate()`, which is the default implementation of `CGCXXABI::emitTerminateForUnexpectedException`. - Remove `WebAssembly::ensureSingleBBTermPads() function and `WebAssemblyHandleEHTerminatePads` pass, because terminate pads are already `catch_all` now (because they don't need the exception pointer) and we don't need these transformations anymore. - Change tests to use `std::terminate` directly. Also removes tests that tested `LateEHPrepare::ensureSingleBBTermPads` and `HandleEHTerminatePads` pass. - Drive-by fix: Add some function attributes to EH intrinsic declarations Fixes https://github.com/emscripten-core/emscripten/issues/13582. Reviewed By: dschuff, tlively Differential Revision: https://reviews.llvm.org/D97834 -
William S. Moses authored
This reverts commit 51bd42ef.
-
Sanjay Patel authored
This code assumed that FP math was only permissable if it was fully "fast", so it hard-coded "fast" when creating new instructions. The underlying code already allows matching recurrences/reductions that are only "reassoc", so this change should prevent the potential miscompile seen in the test diffs (we created "fast" ops even though none existed in the original code). I don't know if we need to create the temporary IRBuilder objects used here, so that could be follow-up clean-up. There's an open question about whether we should require "nsz" in addition to "reassoc" here. InstCombine uses that combo for its reassociative folds, but I think codegen is not as strict.
-
William S. Moses authored
Enable Attributor's heap-to-stack to lower unbounded allocations given a max size of -1 Differential Revision: https://reviews.llvm.org/D97873
-
Reid Kleckner authored
Use a WeakTrackingVH to cope with the stmt emission logic that cleans up unreachable blocks. This invalidates the reference to the deferred replacement placeholder. Cope with it. Fixes PR25102 (from 2015!)
-
dfukalov authored
Preparation to make AliasResult scoped enumeration. Reviewed By: nikic Differential Revision: https://reviews.llvm.org/D97973
-
Jay Foad authored
The hazard where a VMEM reads an SGPR written by a VALU counts as a data dependency hazard, so no nops are required on GFX10. Tested with Vulkan CTS on GFX10.1 and GFX10.3. Differential Revision: https://reviews.llvm.org/D97926
-
LLVM GN Syncbot authored
-
Nico Weber authored
-
Eric Schweitz authored
-
KareemErgawy-TomTom authored
To unify the naming scheme across all ops in the SPIR-V dialect, we are moving from spv.camelCase to spv.CamelCase everywhere. Reviewed By: antiagainst Differential Revision: https://reviews.llvm.org/D97919
-
Martin Storsjö authored
Differential Revision: https://reviews.llvm.org/D89945
-
KareemErgawy-TomTom authored
To unify the naming scheme across all ops in the SPIR-V dialect, we are moving from `spv.camelCase` to `spv.CamelCase` everywhere. Reviewed By: antiagainst Differential Revision: https://reviews.llvm.org/D97917
-
Jinsong Ji authored
To avoid assembler errors. Reviewed By: sfertile Differential Revision: https://reviews.llvm.org/D97418
-
KareemErgawy-TomTom authored
To unify the naming scheme across all ops in the SPIR-V dialect, we are moving from spv.camelCase to spv.CamelCase everywhere. Differential Revision: https://reviews.llvm.org/D97920
-
Philip Reames authored
BasicAA knows how to analyze phis, but to control compile time, we're fairly limited in doing so. This patch loosens that restriction just slightly when there is exactly one phi input (after discounting induction variable increments). The result of this is that we can handle more cases around nested and sibling loops with pointer induction variables. A few points to note. * This is deliberately extremely restrictive about recursing through at most one input of the phi. There's a known general problem with BasicAA sometimes hitting exponential compile time already, and this patch makes every effort not to compound the problem. Once the root issue is fixed, we can probably loosen the restrictions here a bit. * As seen in the test file, we're still missing cases which aren't *directly* based on phis (e.g. using the indvar increment). I believe this to be a separate problem and am going to explore this in another patch once this one lands. * As seen in the test file, this results in the unfortunate fact that using phivalues sometimes results in worse quality results. I believe this comes down to an oversight in how recursive phi detection was implemented for phivalues. I'm happy to tackle this in a follow up change. Differential Revision: https://reviews.llvm.org/D97401
-
River Riddle authored
Now that attributes can be generated using ODS, we can move the builtin attributes as well. This revision removes a majority of the builtin attributes with a few left for followup revisions. The attributes moved to ODS in this revision are: AffineMapAttr, ArrayAttr, DictionaryAttr, IntegerSetAttr, StringAttr, SymbolRefAttr, TypeAttr, and UnitAttr. Differential Revision: https://reviews.llvm.org/D97591
-
River Riddle authored
The value type of the attribute can be specified by either overriding the typeBuilder field on the AttrDef, or by providing a parameter of type `AttributeSelfTypeParameter`. This removes the need to define custom storage class constructors for attributes that have a value type other than NoneType. Differential Revision: https://reviews.llvm.org/D97590
-
Louis Dionne authored
-
George Balatsouras authored
Add more expectations in vector.ll and select.ll based on command-line option combinations. Also, remove hard-coded shadow width references to enable fast8 transition. Reviewed By: stephan.yichao.zhao Differential Revision: https://reviews.llvm.org/D97903
-
Francis Visoiu Mistrih authored
This enhances the auto-init remark with information about the variable that is auto-initialized. This is based of debug info if available, or alloca names (mostly for development purposes). ``` auto-init.c:4:7: remark: Call to memset inserted by -ftrivial-auto-var-init. Memory operation size: 4096 bytes.Variables: var (4096 bytes). [-Rpass-missed=annotation-remarks] int var[1024]; ^ ``` This allows to see things like partial initialization of a variable that the optimizer won't be able to completely remove. Differential Revision: https://reviews.llvm.org/D97734 -
Petar Avramovic authored
This is recommit of 4c8fb7dd. MIR in one unit test had mismatched types. For vectors we consider a bit as known if it is the same for all demanded vector elements (all elements by default). KnownBits BitWidth for vector type is size of vector element. Add support for G_BUILD_VECTOR. This allows combines of urem_pow2_to_mask in pre-legalizer combiner. Differential Revision: https://reviews.llvm.org/D96122
-
Nicolas Guillemot authored
This reverts commit 7479a2e0. This commit causes compile errors on clang-x64-windows-msvc, so I'm reverting the patch for now. For reference, the error in question is: ``` error C2280: 'llvm::raw_ostream_iterator<char,char> &llvm::raw_ostream_iterator<char,char>::operator =(const llvm::raw_ostream_iterator<char,char> &)': attempting to reference a deleted function note: compiler has generated 'llvm::raw_ostream_iterator<char,char>::operator =' here note: 'llvm::raw_ostream_iterator<char,char> &llvm::raw_ostream_iterator<char,char>::operator =(const llvm::raw_ostream_iterator<char,char> &)': function was implicitly deleted because 'llvm::raw_ostream_iterator<char,char>' has a data member 'llvm::raw_ostream_iterator<char,char>::OutStream' of reference type ```
-
Benjamin Kramer authored
-
Philip Reames authored
-
Philip Reames authored
-
Philip Reames authored
-
Gui Andrade authored
This change adds a new IR noundef attribute, which denotes when a function call argument or return val may never contain uninitialized bits. In MemorySanitizer, this attribute enables optimizations which decrease instrumented code size by up to 17% (measured with an instrumented build of clang) . I'll introduce the change allowing msan to take advantage of this information in a separate patch. Differential Revision: https://reviews.llvm.org/D81678
-
Mitch Phillips authored
We're having flaky failures on this test on the sanitizer slow buildbot. Not per-run flaky, but it'll be green for a while, then red for a while. I suspect that changes in codegen are causing the LLVM_VP_MAX_NUM_VALS_PER_SITE=150 to be above and below the limit sporadically. The limit on my machine using lld and a non-bootstrapped compiler is 175, but the bot uses GNU ld and ld.gold at different points, which could be affecting behaviour. Change this threshold to LLVM_VP_MAX_NUM_VALS_PER_SITE=130 in order to try and get it below the failure point, at least for the foreseeable future. http://lab.llvm.org:8011/#/builders/37/builds/2744
-
Philip Reames authored
-
Jens Massberg authored
[clang-tidy] Add options to describe individual core increments to readability-function-cognitive-complexity check. Often you are only interested in the overall cognitive complexity of a function and not every individual increment. Thus the flag 'DescribeBasicIncrements' is added. If it is set to 'true', each increment is flagged. Otherwise, only the complexity of function with complexity of at least the threshold are flagged. By default 'DescribeBasisIncrements' is set to 'true', which is the original behavior of the check. Added a new test for different flag combinations. (The option to ignore macros which was original part of this patch will be added in another path) Reviewed By: lebedev.ri Differential Revision: https://reviews.llvm.org/D96281
-
River Riddle authored
This function simplifies calling the getChecked methods on Attributes and Types from within the parser, and removes any need to use `getEncodedSourceLocation` for these methods (by using an SMLoc instead). This is much more efficient than using an mlir::Location, as the encoding process to produce an mlir::Location is inefficient and undesirable for parsing (locations used during parsing should not persist afterwards unless otherwise necessary). Differential Revision: https://reviews.llvm.org/D97900
-
Jez Ng authored
Since multiple dylibs can be defined in one TBD, this is necessary to avoid confusion. Reviewed By: #lld-macho, oontvoo Differential Revision: https://reviews.llvm.org/D97905
-
Jez Ng authored
Previously, we were loading re-exports without checking whether they were compatible with our target. Prior to {D97209}, it meant that we were defining dylib symbols that were invalid -- usually a silent failure unless our binary actually used them. D97209 exposed this as an explicit error. Along the way, I've extended our TAPI compatibility check to cover the platform as well, instead of just checking the arch. To this end, I've replaced MachO::Architecture with MachO::Target in our Config struct. Reviewed By: #lld-macho, oontvoo Differential Revision: https://reviews.llvm.org/D97867 -
Jez Ng authored
The reexport-nested-libs test added in D97438 was a bit wonky. First, it was linking against libReexportSystem.tbd which targets the iOS simulator, and which in turn attempted to re-export the iOS simulator's libSystem. However, due to the way `-syslibroot` works, it was actually re-exporting the macOS libSystem. As a result, the test was not actually able to resolve the symbols in the desired libSystem. I'm guessing that @oontvoo was confused by this and therefore included those symbols in libReexportSystem.tbd itself. But this means that the test wasn't actually testing the resolution of re-exported symbols (though it did at least verify that the re-exported libraries could be located). After some consideration, I figured that stub-link.s could be extended to cover what reexport-nested-libs.s was attempting to do. The test targets macOS, so we only have one `-syslibroot` and no chance of confusion. Reviewed By: #lld-macho, oontvoo Differential Revision: https://reviews.llvm.org/D97866
-
Jez Ng authored
Suppose we are linking against libFoo, which re-exports the implicitly-bound libSystem, which in turn re-exports some non-explicitly-bound library like `/usr/lib/system/libsystem_c.dylib`. Then any bindings we have to a symbol in libsystem_c should use libSystem (and not libFoo) as the umbrella library. Reviewed By: #lld-macho, smeenai Differential Revision: https://reviews.llvm.org/D97865
-
Haowei Wu authored
This change adds '-use-interfacestub' option to allow llvm-ifs to use InterfaceStub lib when generating ELF binary. Differential Revision: https://reviews.llvm.org/D94461
-
Siva Chandra Reddy authored
-
Med Ismail Bennani authored
When using `OptionGroupPythonClassWithDict` options in an `OptionGroup` with other `Options`, it can happen that the combinaison of some options of each group makes the command invalid. To solve that issue, this patch adds a bitmask argument to the `OptionGroupPythonClassWithDict` constuctor that is used to mark each option as required (or not). If the `required_options` bitmask isn't passed to the constructor, the class will keep its default behaviour, making the `--script-class` and `--python-function` required. rdar://65508855 Differential Revision: https://reviews.llvm.org/D97910 Signed-off-by:
Med Ismail Bennani <medismail.bennani@gmail.com>
-