Skip to content

Conversation

@pull
Copy link

@pull pull bot commented Oct 19, 2020

See Commits and Changes for more details.


Created by pull[bot]. Want to support this open source service? Please star it : )

eeckstein and others added 15 commits October 16, 2020 16:32
This refactoring removes a lot of special-casing in collectLoads and also makes tryOptimizeStoreIntoTemp simpler.
It's a NFC.
It's sufficient to pass the operand instead of the operand, the user and the operand value.
NFC.
…to checkNoSourceModification

... where it belongs.
This is mostly refactoring, but it also fixes a bug: we don't recurse into a begin_access in collectLoads.
If there is an apply in such a scope, the mayWrite-check wouldn't be done.
In checkNoSourceModification all instructions are visited, so the check is always done.
Consider the related end_access instructions as uses to correctly mark the end of the lifetime of the temporary.
This fixes a miscompile in case there is a modification of the copy-source between an begin_access and end_access.
load [take] was not considered as a use and it was not detected if it's in a different basic block.
This fixes a miscompile in case there is a modification of the copy-source before a load [take].

rdar://problem/69757314
... and use that API in FullApplySite::insertAfterInvocation.

Also change FullApplySite::insertAfterInvocation/insertAfterFullEvaluation to directly pass a SILBuilder instead of just an insertion point to the callback.
This makes more sense (given the function names) and simplifies the usages.

It's a NFC.
…dr [take]

Instead of reusing the existing destroy_addr (or load/copy_addr [take]) of the temporary, insert a new destroy_addr - at the correct location.
This fixes a memory lifetime failure in case the copy-source is modified (actually: re-initialized) after the last use of the temporary: E.g. (before copy elimination):

copy_addr [take] %src to [initialization] %temp
%x = load %temp // last use of %temp
store %y to [init] %src
destroy_addr %temp

The correct location for the destroy_addr is after the last use (after copy elimination):

%x = load %src
destroy_addr %src
store %y to [init] %src

rdar://problem/69757314
…temporary.

This fixes a memory lifetime failure.
…n in non-OSSA mode.

In OSSA, memory locations are always destroyed in a "visible" way, e.g. with destroy_addr or load [take].
…-commandline-test

[XFAIL] Disable validation-test/stdlib/CommandLine.swift (70423908)
Fix several correctness issues in TempRValueOpt
…infinity (swiftlang#34339)

Previously, overflow and underflow both caused this to return `nil`, which causes several problems:
* It does not distinguish between a large but valid input and a malformed input.  `Float("3.402824e+38")` is perfectly well-formed but returns nil
* It differs from how the compiler handles literals.  As a result, `Float(3.402824e+38)` is very different from `Float("3.402824e+38")`
* It's inconsistent with Foundation Scanner()
* It's inconsistent with other programming languages

This is exactly the same as swiftlang#25313

Fixes rdar://problem/36990878
@pull pull bot merged commit acebbdc into swiftwasm Oct 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants