These are chat archives for halide/Halide

28th
Nov 2017
ronlieb
@ronlieb
Nov 28 2017 00:57
Hi Folks, i am seeing a failure building camera_pipe after the most recent commit.
make: * No rule to make target bin/Demosaic.o', needed bybin/process'. Stop.
Dillon Sharlet
@dsharletg
Nov 28 2017 02:08
just pushed what should fix it
ronlieb
@ronlieb
Nov 28 2017 12:33
it did ,thx
Suyog
@suyogsarda_twitter
Nov 28 2017 15:40

Hi All, i was looking at issue 2317 (halide/Halide#2317) where input.dim(0).set_min(0) was resulting in slower code on CPU. Further digging into code and some experiment showed that slowness is only due to input.dim(0).set_min(0) and not due to input.dim(1).set_min(0).

In the codegen, i see some checks and asserts for "halide_buffer_is_bounds_query" and these are inserted on CPU side always. Even if the schedule is offloaded to Hexagon, the asserts are always inserted in CPU code. Hence the slowness is always observed on CPU schedule, but not on Hexagon.

Q - For schedules offloaded to Hexagon, even if the asserts are on CPU side, why isn't slowness observed? I assume we are measuring time which involves the CPU to Hexagon and back offload time too. Any idea?

Andrew Adams
@abadams
Nov 28 2017 16:11
It'd have to be a really small pipeline for that assert to matter - e.g. processing an 8x8 image
It's an inlined comparison of two of the input buffer fields to zero - should be perfectly branch-predicted too
The no_asserts-no_bounds_query target flags turns off all that code, so you can try those for testing.
Suyog
@suyogsarda_twitter
Nov 28 2017 16:24
Thanks i will try that. However, the effect is observed for every test case though (and size of image is large enough). Also, in the code generated, CPU code differs only on those asserts accompanied by some bunch of mov instructions at the end of computations for that function, while for schedules on hexagon, code generated is exactly same. Hence my guess was those asserts for slowness on CPU code.
Andrew Adams
@abadams
Nov 28 2017 16:27
@steven-johnson windows builds are now all failing tests (now that we're running them). Can't tell if it's real, or a build config issue. I see cmake reporting steps as failed, but don't see any indication of what failed.
It's not doing something dumb like assuming the word "error" in the output means a failure is it? I recall that being an issue
Steven Johnson
@steven-johnson
Nov 28 2017 16:29
Well, that's progress I guess... Will investigate when I get in today. It's almost certainly a build config issue unless something has injected a platform specific failure in the last week or so as these targets worked correctly on my local box.
Suyog
@suyogsarda_twitter
Nov 28 2017 16:52
@abadams arm-64-android-no_asserts-no_bounds_query didn't produce faster code. Problem seems something else then
Steven Johnson
@steven-johnson
Nov 28 2017 16:59
looking at windows failure log now — there are only a handful of failures but the errors aren’t really explicated, gonna have to try to replicate individually by remoting into one of the windows buildbots. (My personal Windows box is down again and I’m in different office today, yay windows)
(given that it’s only a handful, it’s possible these are legit errors that have crept in over the past two weeks of non-testing rather than config stuff)
Steven Johnson
@steven-johnson
Nov 28 2017 17:28
@abadams: are the two Windows buildbots interchangeable from a build target standpoint?
Andrew Adams
@abadams
Nov 28 2017 17:28
yes
Steven Johnson
@steven-johnson
Nov 28 2017 17:32
Unsurprisingly, trying to Chromote into them from a GBus is unusably slow. (Not even glacial, this is more tectonic, several-seconds-to-respond-to-keystroke-slow.) I’m just gonna assume that ssh’ing into them isn’t a thing. Will have to wait until I get in to debug further :-(
Andrew Adams
@abadams
Nov 28 2017 17:33
I find them to be super-slow until you turn off the buildbot, just due to windows process priority issues.
But the gbus certainly won't help :)
Should really remember to lower the process priority of the shell the buildbot is launched in, so that everything it spawns is also low-priority.
Steven Johnson
@steven-johnson
Nov 28 2017 17:37
ahhh yes stopping the worker makes it usable :)
Steven Johnson
@steven-johnson
Nov 28 2017 18:07
I hate CMake with the fury of a million exploding suns
Andrew Adams
@abadams
Nov 28 2017 18:08
Because you discovered what was wrong and it's dumb, or because you can't figure out what's wrong because it sucks?
Steven Johnson
@steven-johnson
Nov 28 2017 18:08
If you run something via add_custom_target(), it considers a nonzero error code to be failure, as it should
but
for its MSVC backend
it also considers any output containing “error” and “:” on the same line to mean “this is should be considered an error"
So if you see, oh, I dunno, “Expected error: foo”….
Andrew Adams
@abadams
Nov 28 2017 18:09
Hah! So it was the dumb thing I was thinking of.
Steven Johnson
@steven-johnson
Nov 28 2017 18:09
this is just so bad and wrong I don’t know where to begin
And googling for it, it’s been like this since at least 2009 or so
I mean, this isn’t just a leaky abstraction, this is like building Hoover Dam out of kleenex
How we ended up with CMake as the ‘best’ portable C++ build system should be an embarassment to the entire C++ community
rant rant rant
Andrew Adams
@abadams
Nov 28 2017 18:12
I do not disagree
Steven Johnson
@steven-johnson
Nov 28 2017 18:13
(looking at cmake docs, or should I say “docs”, to see if there’s at least an option to defeat this “feature”. I’ll bet you a dollar there isn’t.)
Steven Johnson
@steven-johnson
Nov 28 2017 18:27
If there’s a way to defeat it, my google-fu is unable to find it. I’m going to just go change the relevant tests to not output the text.
(extra fun: apparently it also treats the text “warning.+:” specially, but when I run the test singly, it just produces a warning, whereas with the batch build, it is lumped in with the errors.)
Steven Johnson
@steven-johnson
Nov 28 2017 18:41
Pushed a fix that is working locally. (Why this didn’t bite me when testing on my local box is concerning, however; perhaps the behavior varies between CMake versions? I’ll investigate once I can restart my Windows box.)
Steven Johnson
@steven-johnson
Nov 28 2017 19:28
(FYI: you’d think you could fix this by just redirecting stdout/stderr in the add_custom_target COMMAND argument in cmake, but apparently redirection doesn’t work reliably for that command in cmake, for no obvious reason.)
I think the errors are now fixed. (There are still some spurious warnings but I’ll deal with those in a subsequent PR.) WinWorker #2 restarted.
Andrew Adams
@abadams
Nov 28 2017 22:58
Looks like generators and tutorials are still failing
Steven Johnson
@steven-johnson
Nov 28 2017 22:58
Been in a meeting, thanks for the heads up, will revisit shortly
Andrew Adams
@abadams
Nov 28 2017 22:58
We weren't previously testing tutorials
(on windows)
So there may be real problems there
Steven Johnson
@steven-johnson
Nov 28 2017 22:58
True. I added to match make behavior.
Steven Johnson
@steven-johnson
Nov 28 2017 23:25
two dumb mistakes. fixes pushed, restarting shortly