This bugzilla service is closed. All entries have been migrated to https://gitlab.com/libeigen/eigen
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: 2019-12-04 12:46 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.
Comment 5 Nobody 2019-12-04 12:46:35 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to gitlab.com's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.com/libeigen/eigen/issues/697.

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