Fixed a bug in const folding, where an inst that's not of type SingleValueInstruction gets added to the worklist #18272
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
SingleValueInstruction gets added to the worklist, causing cast assert at https://github.com/apple/swift/blame/master/lib/SILOptimizer/Utils/ConstantFolding.cpp#L1585. One such example inst is the following, which produces a SILValue of type MultipleValueInstructionResult, so ValueBase::getDefiningInstruction() still returns a valid inst for it, even though that graph_op inst is not a SingleValueInstruction. ``` %94 = graph_op "Fill,i,i"(%73 : $TensorHandle<Int32>, %85 : $TensorHandle<Float>) {T: $Float, index_type: $Int32, __device: "/device:CPU:0"} : $TensorHandle<Float> // users: %1581, %100, %98, %97, %99, %103, %106, %111, %113, %118, %755 ```
|
@swift-ci please test tensorflow |
mhong
requested review from
devincoughlin,
eeckstein and
lattner
and removed request for
devincoughlin and
eeckstein
|
I can upstream this fix later if that makes sense. |
|
I didn't have a simpler test case, but the fix is motivated by a crash in the AutoEncoder model, and I verified the fix with that model as follows. The compilation finishes. When I run the code as I got: This is probably due to my incorrect env, and I guess would not occur if we build and run this model with the proper S4TF toolchain. |
|
I built a toolchain, and compiled the model code with: But the above runtime error still occurs. @rxwei, are you seeing this as well? |
|
From google search, this error appears to come from python (e.g. MDAnalysis/mdanalysis#1739). Any ideas? |
|
This should be a python error. On macOS, removing brewed python would solve the problem. Not sure about Linux though. |
lattner
approved these changes
Jul 27, 2018
|
@dan-zheng Dan, can you try if you can reproduce this python error on Linux? Thanks. |
|
@mhong I wasn't able to reproduce the error on a fresh toolchain build (swift 37864e1, swift-models tensorflow/swift-models@40c239b). I ran the following and it worked fine:
|
|
@dan-zheng, running My error trace: |
|
You need to install matplotlib. |
|
The error is Incidentally, I did find another bug with the data loading logic (using |
|
The matplotlib error appears a red herring -- I got that only when I activate a certain conda env. Once I deactivate that, I'm getting back to the |
mhong
pushed a commit
to mhong/swift
that referenced
this pull request
Jul 27, 2018
SingleValueInstruction gets added to the worklist, causing cast assert at https://github.com/apple/swift/blame/master/lib/SILOptimizer/Utils/ConstantFolding.cpp#L1585. One such example inst is the following (in the tensorflow branch), which produces a SILValue of type MultipleValueInstructionResult, so ValueBase::getDefiningInstruction() still returns a valid inst for it, even though that graph_op inst is not a SingleValueInstruction. ``` %94 = graph_op "Fill,i,i"(%73 : $TensorHandle<Int32>, %85 : $TensorHandle<Float>) {T: $Float, index_type: $Int32, __device: "/device:CPU:0"} : $TensorHandle<Float> ``` The same fix has been merged into the tensorflow branch: swiftlang#18272
mhong
pushed a commit
to mhong/swift
that referenced
this pull request
Jul 31, 2018
MultipleValueInstructionResult, which got fixed in swiftlang#18272. Confirmed this test crashes without that fix.
mhong
pushed a commit
that referenced
this pull request
Aug 1, 2018
MultipleValueInstructionResult, which got fixed in #18272. Confirmed this test crashes without that fix.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This causes cast assert at https://github.com/apple/swift/blame/master/lib/SILOptimizer/Utils/ConstantFolding.cpp#L1585.
One such example inst is the following, which produces a SILValue of type
MultipleValueInstructionResult, so ValueBase::getDefiningInstruction() still
returns a valid inst for it, even though that graph_op inst is not a SingleValueInstruction.