Update tensorflow and llvm submodules.#762
Update tensorflow and llvm submodules.#762stellaraccident wants to merge 1 commit intoiree-org:masterfrom
Conversation
stellaraccident
commented
Feb 14, 2020
- tensorflow -> head
- llvm -> head (not tensorflow version because incompatible)
- llvm bazel build files: imported from tensorflow and manually patched
- adapted to new dialect registration scheme by adding calls to the registration method
- all tests pass on my linux vm with bazel (sans Vulkan due to no graphics card)
- will give cmake a shot in a minute but this has been a lot already and need to sign off
* tensorflow -> head * llvm -> head (not tensorflow version because incompatible) * llvm bazel build files: imported from tensorflow and manually patched * adapted to new dialect registration scheme by adding calls to the registration method * all tests pass on my linux vm with bazel (sans Vulkan due to no graphics card) * will give cmake a shot in a minute but this has been a lot already and need to sign off
|
@stellaraccident I can take over and give CMake a try based on what you already did. |
|
Thanks - I was just about to email you. I really need to sign off now but
was poking CMake a bit...
The primary thing that changed upstream is how they are doing dialect
registration. There is now an explicit mlir::registerAllDialects() in any
binary that needs them (from mlir/InitAllDialects.h). It looks like this is
an intermediate state, still doing static registration in some way that I
haven't untangled how to depend on properly from the CMake side. There is
an MLIRAllDialects target which is sufficient to get enough headers to
compile, but then linking fails.
One issue is that this new mechanism pulls in all MLIR dialects, whether we
need them or not, and it appears that the ones we weren't using are the
ones that then fail on link (FxpMathOps is one that jumps out and that I
clue in on). We will probably eventually want our own, properly scoped
list, but for now, just pulling them all in from upstream is better than
being broken (which the most recent integrate was causing). I think we just
need every binary to depend on the full set of dialects -- maybe a common
list somewhere so we can at least not repeat ourselves?
I'd really love to get the cmake version working again and stable so I can
use it as a main mechanism of developing... believe it or not, my CMake
builds are ~3-5x faster than corresponding Bazel builds and build the whole
project in about 2 minutes (CMake maxes out all 64 cores in my machine,
whereas Bazel rarely utilizes more than ~20). I feel like Bazel claimed to
have required all of the complex build machinery for parallelism... and
does not seem to deliver on local builds. I bet it is rare that big
projects have an apples to apples comparison of the same build on both
systems.
…On Thu, Feb 13, 2020 at 10:32 PM Marius Brehler ***@***.***> wrote:
@stellaraccident <https://github.com/stellaraccident> I can take over and
give CMake a try based on what you already did.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#762>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AADYVAGSOGTO6DLOKAW3UIDRCY3IBANCNFSM4KVBRTVA>
.
|
|
+Lei Zhang <antiagainst@google.com> who was oncall this week for MLIR
integration issues and may have more context on the static registration
stuff.
On Thu, Feb 13, 2020 at 10:50 PM Stella Laurenzo <laurenzo@google.com>
wrote:
… Thanks - I was just about to email you. I really need to sign off now but
was poking CMake a bit...
The primary thing that changed upstream is how they are doing dialect
registration. There is now an explicit mlir::registerAllDialects() in any
binary that needs them (from mlir/InitAllDialects.h). It looks like this is
an intermediate state, still doing static registration in some way that I
haven't untangled how to depend on properly from the CMake side. There is
an MLIRAllDialects target which is sufficient to get enough headers to
compile, but then linking fails.
One issue is that this new mechanism pulls in all MLIR dialects, whether
we need them or not, and it appears that the ones we weren't using are the
ones that then fail on link (FxpMathOps is one that jumps out and that I
clue in on). We will probably eventually want our own, properly scoped
list, but for now, just pulling them all in from upstream is better than
being broken (which the most recent integrate was causing). I think we just
need every binary to depend on the full set of dialects -- maybe a common
list somewhere so we can at least not repeat ourselves?
I'd really love to get the cmake version working again and stable so I can
use it as a main mechanism of developing... believe it or not, my CMake
builds are ~3-5x faster than corresponding Bazel builds and build the whole
project in about 2 minutes (CMake maxes out all 64 cores in my machine,
whereas Bazel rarely utilizes more than ~20). I feel like Bazel claimed to
have required all of the complex build machinery for parallelism... and
does not seem to deliver on local builds. I bet it is rare that big
projects have an apples to apples comparison of the same build on both
systems.
On Thu, Feb 13, 2020 at 10:32 PM Marius Brehler ***@***.***>
wrote:
> @stellaraccident <https://github.com/stellaraccident> I can take over
> and give CMake a try based on what you already did.
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <#762>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AADYVAGSOGTO6DLOKAW3UIDRCY3IBANCNFSM4KVBRTVA>
> .
>
|
|
Intermediate status:
|
|
I opened #763 as a follow up and to discuses my modifications. My branch might need rebasing (this and an earlier PR of mine are merged), but if you like feel free to take back over again. |
@OliverUrbann We need a more powerfull build machine as well. My 16GB RAM are not sufficient to run six linker processes in parallel. It actually resulted in eating up nearly my whole swap and an unusable system. 64 cores and appropriate RAM sounds adequate ;) |
|
Yeah right now we can rely on the gigantic |
|
This needs some surgery upstream where we have stricter checks on depending on the rule that exports the headers you use. |
* tensorflow -> head * llvm -> head (not tensorflow version because incompatible) * llvm bazel build files: imported from tensorflow and manually patched * adapted to new dialect registration scheme by adding calls to the registration method * all tests pass on my linux vm with bazel (sans Vulkan due to no graphics card) * will give cmake a shot in a minute but this has been a lot already and need to sign off Closes #762 FUTURE_COPYBARA_INTEGRATE_REVIEW=#762 from stellaraccident:bumpllvm_20200213 2809231 PiperOrigin-RevId: 295168058
* tensorflow -> head * llvm -> head (not tensorflow version because incompatible) * llvm bazel build files: imported from tensorflow and manually patched * adapted to new dialect registration scheme by adding calls to the registration method * all tests pass on my linux vm with bazel (sans Vulkan due to no graphics card) * will give cmake a shot in a minute but this has been a lot already and need to sign off Closes #762 FUTURE_COPYBARA_INTEGRATE_REVIEW=#762 from stellaraccident:bumpllvm_20200213 2809231 PiperOrigin-RevId: 295168058
* tensorflow -> head * llvm -> head (not tensorflow version because incompatible) * llvm bazel build files: imported from tensorflow and manually patched * adapted to new dialect registration scheme by adding calls to the registration method * all tests pass on my linux vm with bazel (sans Vulkan due to no graphics card) * will give cmake a shot in a minute but this has been a lot already and need to sign off Closes #762 FUTURE_COPYBARA_INTEGRATE_REVIEW=#762 from stellaraccident:bumpllvm_20200213 2809231 PiperOrigin-RevId: 295168058
This is a adds changes to get the CMake build working again. It includes the commits of #744 and #762, which can be removed from this PR after those made it to the master, followed by a rebase. Closes #763 FUTURE_COPYBARA_INTEGRATE_REVIEW=#763 from iml130:bumpllvm_20200213 b584647 PiperOrigin-RevId: 295192350
This is a adds changes to get the CMake build working again. It includes the commits of #744 and #762, which can be removed from this PR after those made it to the master, followed by a rebase. Closes #763 FUTURE_COPYBARA_INTEGRATE_REVIEW=#763 from iml130:bumpllvm_20200213 b584647 PiperOrigin-RevId: 295192350
Would you propose to fork |
|
Sorry for the late reply. Yeah I think that makes sense. |
|
Okay, will go for it after I returned to the office from C4ML. I think I will send a PR latest end of the week. |
Follow up to the discussion with @antiagainst started [here](#762 (comment)). This serves as the basis to reduce the number of registered and linked passes. Closes #858 COPYBARA_INTEGRATE_REVIEW=#858 from iml130:opt_main 716c03a PiperOrigin-RevId: 298646127