- User Since
- Dec 19 2014, 12:23 PM (160 w, 5 d)
Mon, Jan 15
Addressed David's comments.
Fri, Jan 12
Thanks for the quick feedback Danny/Davide. I definitely appreciate the point that SimplifyCFG may not be the best place for this kind of transformation. Davide, I assume the dedicated pass you're referring to is GVNSink? I don't think that pass is enabled yet (I haven't been closely following the progress, so I'm not sure what's holding it up at this point), but it's possible GVNSink would indeed catch the cases this patch does. I haven't tested that yet.
Thanks for cleaning this up.
Thu, Jan 11
Mon, Jan 8
Thu, Jan 4
Wed, Dec 27
Tue, Dec 26
Wed, Dec 20
Tue, Dec 19
Thanks David! I'll make those changes before committing.
Dec 14 2017
Addressed David's comments. Thanks again!
Thanks again for the comments, David. I'll update the patch shortly.
Dec 13 2017
Dec 12 2017
Moved demoteCall, previously in D40751, to this patch.
Addressed David's comments. Thanks for the feedback!
Dec 11 2017
Dec 6 2017
Dec 1 2017
Made this patch NFC and split out the functional changes.
One minor comment. Otherwise, this looks fine to me.
Nov 30 2017
Rebased on top of D40658. We now use a common indirect call promotion interface.
Nov 27 2017
Nov 17 2017
Nov 9 2017
- Incorporated the change into the lazy call graph as well. The metadata is represented as ref edges there.
- Added a helper in CallSite to get the functions listed in the metadata.
Oct 31 2017
Oct 27 2017
Oct 26 2017
How hard do you think it would be to update this transformation to reason about vector GEPs rather than bailing out? (I've left some inline comments if you want to give it a try). Having said that, I'm fine with that patch as is. But you should at least add a comment explaining why we're giving up.
Oct 25 2017
I'm abandoning this for now to close the review. Davide/Danny, please feel free to revive this patch if you want. To summarize the main points in the review, we need to work on the cost model more to enable something like this by default.
Oct 16 2017
I updated this patch following the most recent changes to the generic solver, which allow clients to define custom LatticeKeys. I'll email llvm-dev before committing.
Thanks very much for the reviews!
Oct 13 2017
Oct 12 2017
Implement the previously discussed approach using generic LatticeKeys. A few things to note about this version of the patch:
Oct 10 2017
Addressed Danny's comments.
I agree, getStateMapForInstruction is a bit strange. I've been thinking about what to do here. And one of the simplest things I came up with was to just define a custom LatticeKey type like we do a LatticeVal. This would allow the solver to maintain a single map (meaning we no longer need something like getStateMapForInstruction), and give the client the flexibility to separate the kinds of values it cares about.
Oct 9 2017
Thanks very much for all of the reviews! Just to clarify, are you also OK with the dependent patches (the changes in D37353 to make the generic solver interprocedural and the NFC in D37638 that pulls out the IPO utilities)?
Addressed Danny's comments.
Oct 5 2017
Oct 4 2017
Addressed comments from Danny and Davide.
Addressed comments form Danny.
Oct 3 2017
Oct 2 2017
Sep 29 2017
Sep 25 2017
Sep 18 2017
Sep 15 2017
Made untracked values overdefined.
Sep 14 2017
Yes, and I just found your post about this very issue on llvm-dev from many months ago. Thanks!