Is Visual Studio 2013 Optimizing Correctly in the Presence of /Opt:Icf

Is Visual Studio 2013 optimizing correctly in the presence of /OPT:ICF?

This does not seem like a valid optimization according to the draft C++11 standard section 14.8 [temp.fct.spec] says (emphasis mine going forward):

Each function template specialization instantiated from a template has
its own copy of any static variable. [ Example:

template<class T> void f(T* p) {
static T s;
};
void g(int a, char* b) {
    f(&a); // calls f<int>(int*)
    f(&b); // calls f<char*>(char**)
}

Here f(int*) has a static variable s of type int and
f<char*>(char**) has a static variable s of type char*. —end example ]

Since your taking the address of the variable folding them effects observable behavior which would violate the as-if rule.

T.C. points out that /opt:noicf prevents the non-conforming behavior.

Matt McNabb points out that the /OPT (Optimizations) documentation contains the following note:

Because /OPT:ICF can cause the same address to be assigned to
different functions or read-only data members (const variables
compiled by using /Gy), it can break a program that depends on unique
addresses for functions or read-only data members. For more
information, see /Gy (Enable Function-Level Linking).

Which suggests this could be intentional non-conforming behavior. Ben Voigt says in a comment now moved to chat that this indeed means the optimizations can be non-conforming but this points is debatable.

User usr linked to an MS blog post: Introducing ‘/Gw’ Compiler Switch and it says:

Please note, the ICF optimization will only be applied for identical
COMDATs where their address is not taken, and they are read only. If a
data is not address taken, then breaking address uniqueness by ICF
won't lead to any observable difference, thus it is valid and
conformant to the standard.

and a later comment says:

Even though it's on it's own completely standards complaint, when
combined with /Gy potentially breaking behavior can result.

From what I can tell in order for /Gy to effect const variables __declspec(selectany) has to be used but it could be clearer in the documentation.

At minimum we can see that /Gw should not introduce non-conforming behavior but /Gy in combination with /Gw may.

Any way to generate warnings for function-pointer comparisons?

Is there any compiler option or tool (whether part of Visual C++, GCC, Clang, or other) that can analyze my code and tell me where I'm comparing function pointers with each other, like in the code above?

I'm not sure if there exists such a compiler option.

However, there is such a tool. clang-tidy. You can write your own checks for clang-tidy, it's actually remarkably easy if you follow this blog. Specifically, the AST comes with a bunch of matchers already, which should handle the use-case you want.

Something like this seems to work:

binaryOperator(
    anyOf(hasOperatorName("=="), hasOperatorName("!=")),
    hasLHS(ignoringImpCasts(declRefExpr(hasType(functionType())))),
    hasRHS(ignoringImpCasts(declRefExpr(hasType(functionType())))))

Which flags the example in the OP:

fp.cxx:3:25: note: "root" binds here
int main(void) { return test1 == test2; }
                        ^~~~~~~~~~~~~~

---

That works specifically for the OP case, but you actually have to be more explicit to match all the other likely cases:

const auto AnyFunc = ignoringImpCasts(declRefExpr(hasType(anyOf(
    functionType(),
    pointsTo(functionType()),
    references(functionType())))));

Finder->AddMatcher(binaryOperator(
    anyOf(hasOperatorName("=="), hasOperatorName("!=")),
    hasLHS(AnyFunc),
    hasRHS(AnyFunc)).bind("op"), this);

Or something close to that effect.

Is it safe to use the address of a static local variable within a function template as a type identifier?

Yes, it will be correct to an extent. Template functions are implicitly inline, and static objects in inline functions are shared across all translation units.

So, in every translation unit, you will get the address of the same static local variable for the call to type_id<Type>(). You are protected here from ODR violations by the standard.

Therefore, the address of the local static can be used as a sort of home-brewed run-time type identifier.

Why static variable needs to be explicitly defined?

From the beginning of time C++ language, just like C, was built on the principle of independent translation. Each translation unit is compiled by the compiler proper independently, without any knowledge of other translation units. The whole program only comes together later, at linking stage. Linking stage is the earliest stage at which the entire program is seen by linker (it is seen as collection of object files prepared by the compiler proper).

In order to support this principle of independent translation, each entity with external linkage has to be defined in one translation unit, and in only one translation unit. The user is responsible for distributing such entities between different translation units. It is considered a part of user intent, i.e. the user is supposed to decide which translation unit (and object file) will contain each definition.

The same applies to static members of the class. Static members of the class are entities with external linkage. The compiler expects you to define that entity in some translation unit. The whole purpose of this feature is to give you the opportunity to choose that translation unit. The compiler cannot choose it for you. It is, again, a part of your intent, something you have to tell the compiler.

This is no longer as critical as it used to be a while ago, since the language is now designed to deal with (and eliminate) large amount of identical definitions (templates, inline functions, etc.), but the One Definition Rule is still rooted in the principle of independent translation.

In addition to the above, in C++ language the point at which you define your variable will determine the order of its initialization with regard to other variables defined in the same translation unit. This is also a part of user intent, i.e. something the compiler cannot decide without your help.

---

Starting from C++17 you can declare your static members as inline. This eliminates the need for a separate definition. By declaring them in that fashion you effectively tell compiler that you don't care where this member is physically defined and, consequently, don't care about its initialization order.

Do distinct functions have distinct addresses?

5.10 Equality operators [expr.eq]

1 The == (equal to) and the != (not equal to) operators group left-to-right. The operands shall have arithmetic, enumeration, pointer, or pointer to member type, or type std::nullptr_t. The operators == and != both yield true or false, i.e., a result of type bool. In each case below, the operands shall have the same type after the specified conversions have been applied.

2 If at least one of the operands is a pointer, pointer conversions (4.10) and qualification conversions (4.4) are performed on both operands to bring them to their composite pointer type (Clause 5). Comparing pointers is defined as follows: Two pointers compare equal if they are both null, both point to the same function, or both represent the same address (3.9.2), otherwise they compare unequal.

Let's take the last bit-for-bit:

1. Two null pointers compare equal.
   
   Good for your sanity.
2. Two pointers to the same function compare equal.
   
   Anything else would be extremely surprising.
   
   It also means that only one out-of-line version of any inline-function may ever have its address taken, unless you want to make function-pointer comparisons prohibitively complicated and expensive.
3. Both represent the same address.
   
   Now that one is what it's all about. Dropping this and reducing if and only if to a simple if would leave it to interpretation, but that's a clear mandate to make any two functions identical, as long as it does not otherwise change observable behavior of a conformant program.

---

Related Topics