New user self-registration is disabled due to spam. Please email eigen-core-team @ lists.tuxfamily.org if you need an account.
Before reporting a bug, please make sure that your Eigen version is up-to-date!
Bug 697 - Change order of multiple inheritance
Summary: Change order of multiple inheritance
Status: RESOLVED FIXED
Alias: None
Product: Eigen
Classification: Unclassified
Component: General (show other bugs)
Version: 3.2
Hardware: x86 - 32-bit Windows
: Normal Internal Design
Assignee: Nobody
URL:
Whiteboard:
Keywords: JuniorJob
Depends on:
Blocks: 3.3
  Show dependency treegraph
 
Reported: 2013-11-02 08:54 UTC by panda-34
Modified: 2014-12-02 16:04 UTC (History)
3 users (show)



Attachments

Description panda-34 2013-11-02 08:54:50 UTC
In a typical use of multiple inheritance in Eigen:
class CwiseBinaryOp : internal::no_assignment_operator, public CwiseBinaryOpImpl<...>
sometimes the dummy class (no_assignment_operator) comes first, which leads to suboptimal code generation by Intel Compiler. Every time it needs type casting (which is always) it injects code for null-pointer adjustment, ( if (this==1) this = 0; ) even for stack variables. Simply by reordering the inheritance order such that the dummy class is the last, this problem goes away (and there's a small but noticeable reduction in generated code size).
Comment 1 Christoph Hertzberg 2014-11-02 17:24:59 UTC
Does anyone see problems with changing that order? (I don't)

Patch welcome!
Comment 2 Gael Guennebaud 2014-11-04 11:45:07 UTC
Sounds good and easy.
Comment 3 Gael Guennebaud 2014-12-02 14:42:14 UTC
https://bitbucket.org/eigen/eigen/commits/460f8913a79c/

A respective unit test would be nice, but I don't how to check that.
Comment 4 Christoph Hertzberg 2014-12-02 16:04:53 UTC
We could check things like
  CwiseBinaryOp<...> A(...);
  CwiseBinaryImpl<...> Abase = A;
  VERIFY((void*)(& A) == (void*)(& Abase)));

However, on gcc this always succeeded, independently of the inheritance order (even after I added a dummy variable into no_assignment_operator), so either this test does not work, or GCC just optimizes empty classes away more aggressively than ICC.

Note You need to log in before you can comment on or make changes to this bug.