[llvm] r175293 - add SCEVParameterRewriter

Sebastian Pop spop at codeaurora.org
Tue Feb 19 12:34:15 PST 2013


Chandler Carruth wrote:
> On Fri, Feb 15, 2013 at 12:55 PM, Sebastian Pop <spop at codeaurora.org> wrote:
> 
> > +
> > +  /// The ScevRewriter takes a scalar evolution expression and copies
> > all its
> > +  /// components. The result after a rewrite is an identical SCEV.
> > +  struct ScevRewriter
> > +    : public SCEVVisitor<ScevRewriter, const SCEV*> {
> >
> 
> This class seems to have a pretty weird design to me... So the base class
> is a CRTP visitor, and this class is then derived again, and many of the
> visit methods are now virtual.
> 
> It seems like the better design would be to use consistent CRTP explicit
> delegation for the entire type hierarchy instead of using a mixture of CRTP
> and virtual dispatch.
> 
> In particular, virtual dispatch is remarkably inefficient for visitor
> patterns where you really want the inliner to be able to form a
> jump-table-loop.

Ok. What about removing the virtual functions like in the attached patch.

Thanks,
Sebastian
-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-remove-virtual.patch
Type: text/x-diff
Size: 9044 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130219/89de7ee4/attachment.patch>


More information about the llvm-commits mailing list