- User Since
- Sep 26 2016, 7:58 AM (208 w, 2 d)
Fri, Sep 18
I landed this on behalf of Gabriel.
Thu, Sep 17
Having some problems after this patch, when building the runtimes for i386, on a 64-bit RHEL7 server, and then running the added libunwind test cases on the same host.
No expert here, and maybe that isn't a valid scenario, but building/running tests like that has worked pretty good before.
Wed, Sep 16
Just want to clarify that my questions regarding "what about debug info" in the earlier patch introducing these phi-type-rewrites actually was about dbg.value rather than the debugloc:s. I kind of wanted to have test cases verifying that the the presence of dbg.value uses didn't impact the optimization (-g invariance). But also to see that the dbg.value uses were handled somehow (either being salvaged in some way or being properly invalidated).
Tue, Sep 15
Fri, Sep 4
Just hoping that those unused functions aren't used by some OOT project. But considering that the API is totally untested (and the unwind part isn't even visible any longer) I think this cleanup is motivated.
Wed, Sep 2
Things seem to link again for us if we set LLVM_ENABLE_TERMINFO=OFF (as a workaround to avoid dependency to libtinfo), but I guess we might lose some color output or something by doing that.
So are we perhaps missing tinfo (and ncurses) in /proj/bbi_twh/wh_bbi/x86_64-Linux2/bbigcc/18.104.22.168-3/crosscompiler/lib64?
I doubt that we should pick up libtinfo from /usr/lib64 here, even if we do not really need to crosscompile llvm-tblgen (tblgen should only execute on the current host anyway).
Tue, Sep 1
I don't really understand the problem situation. Why are you having two source operands if they need to be the same? Can't you ensure that by only having one source operand?
Wed, Aug 26
Aug 25 2020
Should I land this patch for you (including a fix for the long lines)?
@ehjogab : Should I help out landing this patch?
Aug 24 2020
LGTM (except the earlier nit about the comments with different X and Y values on left and right hand side)
I've submitted a PR, https://bugs.llvm.org/show_bug.cgi?id=47296, regarding the debuginfo problem.
Aug 23 2020
Haven't seen any gains from this patch, after having rebased to https://reviews.llvm.org/rGec06b381304140b2553cfdfae5a063f39c5c59ff .
The test I ran included bunch of application code, compiled with -O3, for our OOT target.
Aug 21 2020
Aug 20 2020
I've verified that this resolves https://bugs.llvm.org/show_bug.cgi?id=47136 in the sense that our downstream benchmark result is back to the original result for the tests that had a regression. And without any new regressions. So that is perfect!
I do not have any upstream test case for this, but got failures in our downstream testing.
Aug 19 2020
One minor problem with implementing a reverse transform for targets that prefer sext over zext, e.g. in CodeGenPrepare, is that it will be hard to restore the debug info (considering that we do the right thing here an invalidate metadata uses).
Aug 18 2020
Having some problems with this patch downstream:
Aug 15 2020
Aug 14 2020
Aug 13 2020
Might be that it is some other commit that cause the problem. Looking a bit closer at the jenkins history it seems like the tests passed once after this patch was added. But the next time it failed.
Aug 12 2020
Aug 9 2020
(Just some heads up, in case someone else got the same problem...)
Aug 7 2020
Aug 6 2020
Aug 5 2020
Aug 4 2020
Thanks for the fixes Dave. Will make sure that we start using this in our (downstream) fuzzy testing again.
Aug 3 2020
Jul 31 2020
Jul 29 2020
Jul 27 2020
Jul 26 2020
Jul 25 2020
Jul 24 2020
Jul 21 2020
Having some problems with failing stage 2 tests, that I suspect is related to this change (since the missing files do exist but with ".lto" being part of the name when looking in my test directory after a failed test run). I guess on need to add ".lto" at some more places?
I'm a bit curious about the overall plan here. Some years ago the plan was to remove LiveVariables, and use LiveIntervals instead. And afaik PHIElimination was updated to support both LiveVariables and LiveIntervals, while TwoAddressInstructionPass only got partially updated. So for several years there have been a technical debt here, considering that the passes involved in de-SSA to some degree are supporting both LiveVaribels and LiveIntervals, while only LiveVariables is used in practise (and the support for LiveIntervals may have rotten since it is poorly tested).
Jul 10 2020
Here is a reproducer for the new problem that @uabelho noticed. Might be possible to reduce it further, but I think it at least show the problem: