Skip to content

Conversation

@lattner
Copy link
Contributor

@lattner lattner commented Jul 17, 2018

  • introduce a new pass to clean up instructions instead of having
    all the transformations do it. This needs to be generalized, but is
    a start. This should improve compile time when strict deabstraction
    is on.
  • Refactor a bunch of code into a "createConstTensor" helper in
    TFUtilities. It would be nice for the rest of the compiler that
    produces "Const" nodes to start using this.

The patch to SILType.h has been sent upstream in PR18003 so it has no
SWIFT_ENABLE_TENSORFLOW marker.

 - introduce a new pass to clean up instructions instead of having all the transformations do it. This needs to be generalized, but is a start. This should improve compile time when strict deabstraction is on. - Refactor a bunch of code into a "createConstTensor" helper in TFUtilities. It would be nice for the rest of the compiler that produces "Const" nodes to start using this. The patch to SILType.h has been sent upstream in PR18003 so it has no SWIFT_ENABLE_TENSORFLOW marker.
@lattner lattner added the tensorflow This is for "tensorflow" branch PRs. label Jul 17, 2018
@lattner lattner requested review from mhong and rxwei July 17, 2018 17:52
@lattner
Copy link
Contributor Author

lattner commented Jul 17, 2018

@swift-ci please test tensorflow

// All graph_op's get a device.
attributes.push_back({
context.getIdentifier(DEVICE_ATTR),
SymbolicValue::getString(getDeviceString(deviceConfig.primaryDeviceType),
Copy link

Choose a reason for hiding this comment

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

use ALL_DEVICES here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is intended to be usable by multiple clients. Hard coding ALL_DEVICES doesn't seem general... wouldn't that prevent it from being useful to lowerGraph and other things that need it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We chatted, I know what to do :-)

@lattner
Copy link
Contributor Author

lattner commented Jul 17, 2018

@swift-ci please test and merge tensorflow

@lattner
Copy link
Contributor Author

lattner commented Jul 17, 2018

@swift-ci please test tensorflow and merge

@lattner
Copy link
Contributor Author

lattner commented Jul 17, 2018

@swift-ci please test tensorflow

1 similar comment
@lattner
Copy link
Contributor Author

lattner commented Jul 17, 2018

@swift-ci please test tensorflow

@lattner lattner merged commit 9db53fd into tensorflow Jul 17, 2018
@lattner lattner deleted the deabstraction-refactor1 branch July 17, 2018 21:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

tensorflow This is for "tensorflow" branch PRs.

4 participants