@Nikita Popov what did you run to get the test failure due to the D60846 patch?
Something like build/bin/llvm-lit test/Transforms/LICM
Here's the diff over regenerated checks: https://gist.github.com/nikic/c446e525eb5968e1323ca756bf342ef0
Hmmm I must be doing something wrong... I didn't get that diff... I'll tinker some more
Clean build seems to have done the trick. I get the same result you got from
Should I wait for spatel to re-review the patch before I commit it?
They did say "we should be set after that" and @Nikita Popov has also already approved. I would also argue it isn't a particularly controversial change.
@dlrobertson Should be okay to commit.
So, looks like this isn't as simple as it looked...
I feel like there should be a way to make the phi threading in instsimplify miscompile independently of this patch (there are other things depending on the context instruction), but I can't really come up with anything.
@dlrobertson Do you need help with fixing up the patch?
@Nikita Popov hey, I honestly haven't had time to work on it.
I'm mostly not going to be around for another month or so
Just started a new job and my wife is due this week :smile:
If we need to hand it off to someone else, in the meantime... I'm totally okay with that
(Early) congrats @dlrobertson!