Skip to content

Conversation

@eeckstein
Copy link
Contributor

We don't do memory lifetime analysis for this peephole optimization. Therefore we can't risk sinking instructions with address operands out of the addressed memory's lifetime.

For example:

  %3 = mark_dependence %2 on %1 : $*T  // must not be moved after the destroy_addr
  destroy_addr %1

Fixes a verifier crash
rdar://166240751

We don't do memory lifetime analysis for this peephole optimization.
Therefore we can't risk sinking instructions with address operands out of the addressed memory's lifetime.

For example:
```
  %3 = mark_dependence %2 on %1 : $*T  // must not be moved after the destroy_addr
  destroy_addr %1
```

Fixes a verifier crash
rdar://166240751
@eeckstein eeckstein requested a review from atrick December 12, 2025 15:12
@eeckstein
Copy link
Contributor Author

@swift-ci smoke test

@xedin
Copy link
Contributor

xedin commented Dec 12, 2025

@swift-ci please test source compatibility release

@eeckstein eeckstein enabled auto-merge December 12, 2025 18:12
@eeckstein eeckstein disabled auto-merge December 12, 2025 18:12
Copy link
Contributor

@atrick atrick left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is probably necessary just to quiet the memory verifier until it can recognize all the uses of dependent values as uses of the original address.
Ideally the verifier would not accept the incoming SIL:

  %4 = mark_dependence %0 on %2
  destroy_addr %2
  destroy_value %4

It is illegal because it deinitializes a dependent value after deinitializing the value that it depends on.

What pass created the bad SIL pattern? And can we have a separate bug for that?

@eeckstein
Copy link
Contributor Author

eeckstein commented Dec 13, 2025

good point

What pass created the bad SIL pattern? And can we have a separate bug for that?

This SIL comes directly out of SILGen for an ObjectiveC method call which takes a inout argument. Originally it looks like (after DI):

  %54 = alloc_stack [lexical] [var_decl] $Optional<NSString>, var, name "type", type $Optional<NSString>
  ...
  %57 = begin_access [modify] [static] %54
  ...
  %74 = mark_dependence %73 on %57
  store %74 to [assign] %57

Where the [assign] destroys the original value in %57.

The source code:

                var type: NSString?
                let dictionary = depthData.dictionaryRepresentation(forAuxiliaryDataType: &type)

where depthData is an AVDepthData

@eeckstein eeckstein merged commit 80640f7 into swiftlang:main Dec 13, 2025
4 checks passed
@eeckstein eeckstein deleted the fix-sil-combine branch December 13, 2025 16:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants