[PATCH] D31443: [LTO] Do not reorder global variables unnecessarily during merging

[Rafael Espíndola via llvm-commits]

On 28 March 2017 at 18:29, Gerolf Hoflehner via Phabricator
<reviews at reviews.llvm.org> wrote:
> Gerolf added a comment.
>
> I'd love to see some comments an get your thoughts about a verifier.
>
> Thank you
> Gerolf
>
>
>
> ================
> Comment at: lib/Object/ModuleSymbolTable.cpp:48
>      SymTab.push_back(&GV);
> +  for (Function &F : *M)
> +    SymTab.push_back(&F);
> ----------------
> Please add a comment about why the order of the for loop is relevant and how it relates to source code ordering. Also, could there be a verifier that source code order is preserved?
>
>
> ================
> Comment at: test/LTO/Resolution/X86/globalorder.ll:1
> +; Check that the order of global variables is not unnecessarily
> +; perturbed by LTO during module merging.
> ----------------
> How about 'Check that LTO keeps global variables in source code order'?

No. That is not something we want to do.

In fact, I am a bit opposed to this change just because it is setting
the wrong precedence. The output is not defined to be in any specified
order. Anything trying to optimize the order should operate at a level
lower than llvm IR.

Cheers,
Rafael