Nearly all the changes to this pass have been done while maintaining and
updating other parts of LLVM. LLVM has had another pass, SROA, which
has superseded ScalarReplAggregates for quite some time.
Details
Diff Detail
- Repository
- rL LLVM
Event Timeline
This is great!
lib/Transforms/IPO/PassManagerBuilder.cpp | ||
---|---|---|
205 ↗ | (On Diff #60630) | Indentation? (Also below) |
lib/Transforms/IPO/PassManagerBuilder.cpp | ||
---|---|---|
205 ↗ | (On Diff #60630) | The indentation is fine, phab is just not showing the whitespace change. |
Does SROA pass all the test cases for ScalarRepl? I remember when Davide was looking at IPCP vs IPSCCP (with IPCP seeming to be dead) that there were actually some cases missed by IPSCCP and so IPCP wasn't deleted.
A partial survey indicates that SROA does all that ScalarRepl can do. Some ScalarRepl tests which fail are grep based and thus sensitive to the exact IR produced by the pass, others fail because SROA produces IR with fewer bitcasts (ScalarRepl produces stuff like trunc i160 undef to i32).
In any case, it's pretty moot: nobody uses ScalarRepl which makes it effectively dead.
@karthikthecool
I acknowledge that we could remove those functions but our developer policy for the C API recommends that we take a best-effort approach: http://llvm.org/docs/DeveloperPolicy.html#c-api-changes
I figure that keeping the functions around is a net win.
LGTM. I'm a bit sad to not kill this myself, but you should just kill it with fire while you're here. You've done the hard deletion work. I'm glad to see it go now that there is no real reason to keep it around.
(And FWIW, the two things that held it around for this long were the C API and before SROA switched to always use the DomTree some hypothetical concerns about compile time impact of the SSAUpdater path with the new SROA being worse than with ScalarRepl. The latter has been obviated, and the former seems well addressed here and with the C-API policy changes.)