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

Question

I expect the following program to return 0 all of the time. However with Visual Studio 2013 (Update 4), the program exits 1 in release builds. I'm not sure if this is a bug or if the compiler's optimizer is correct and is relying on some edge behavior. If the CONST macro is turned off, the release exe returns 0. If the optimizer is indeed correct, could I get the reason why it is allowed to emit the code it does?

#if 1
#   define CONST const
#else
#   define CONST
#endif


class TypeId {
public:
    bool operator== (TypeId const & other) const
    {
        return id == other.id;
    }

private:
    TypeId (void const * id)
        : id(id)
    {}

public:
    template <typename T>
    static TypeId Get ()
    {
        static char CONST uniqueMemLoc = 0;
        return TypeId(&uniqueMemLoc);
    }

private:
    void const * id;
};


int main(int, char **)
{
    typedef int A;
    typedef unsigned int B;

    if (TypeId::Get<A>() == TypeId::Get<B>()) {
        return 1;
    }
    return 0;
}

Owlet

I think optimizer is incorrect. – Neutral

Probably something to do with COMDAT folding in MSVC. – Volition

Yep, /opt:noicf fixes it. – Volition

This does not seem valid to me and as @Volition points out this looks like a folding issue, perhaps it is related to Do distinct functions have distinct addresses? which is still an open question. – Biak

does /Za imply /opt:noicf? (if not then perhaps it should) – Nuli

@ShafikYaghmour the MSDN documentation seems to imply that it is intentional: "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)." – Nuli

@MattMcNabb perhaps you are correct, it would be nice to get a confirmation that it is indeed expected behavior. – Biak

This optimization is opt-in because according to an MS blog "it broke an important internal customer". That customer was the CLR. They were using a dummy static variable to ensure that function pointers to identical/empty functions are unique. This is nasty. Don't do this. – Amor

@Amor is it possible to link to that entry? – Biak

@ShafikYaghmour how much more "confirmation" do you need than the MSDN documentation saying that this is what the flag does? – Mezcaline

@ShafikYaghmour blogs.msdn.com/b/vcblog/archive/2013/09/11/… – Amor

@Mezcaline I personally don't think it is obvious that the documentation means non-conforming but since the strong consensus is that this is indeed what it means then I will accept that. – Biak

@MattMcNabb: The linker can't fold what aren't in individual COMDATs. So I do expect /Za to avoid the broken behavior, regardless of whether it sets /OPT:NOICF (which is only important is /OPT:REF is specified, otherwise the default of no /OPT options will have folding disabled as well). – Millennium

@ShafikYaghmour For what it's worth, I agree with you in disagreeing with the strong consensus. I think this is worth filing as a bug. – Nonbelligerent

@Amor wait a second, I just fully read the article and it said Please note, the ICF optimization will only be applied for identical COMDATs where their address is not taken which indicates it does not apply to this case. – Biak

@ShafikYaghmour that makes more sense. Under that clause this would be a conforming optimization. – Amor

Answer 1

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.

Biak

Comments are not for extended discussion; this conversation has been moved to chat. – Demonstrative

Answer 2

No, this optimization does not conform to the C++ standard. The declaration of uniqueMemLoc defines a unique object for each instance of the template, and each object has its own address.

(If you had used a string literal, it would be a different story. The optimization would be valid in that case.)

Weed