Hi,
Recently, I noticed that less number of global variables are merged in global-merge pass and in some global variable, array types are used instead of its original type. For example, [4xi8] with align 4 is used for a i32 global variable. For me, it seems that such pattern is observed after r278338 (Resolution-based LTO API). Can anyone give me little bit more insight if such type change was intended? If then why such uses of array type is needed?
Thanks,
Jun
Hi,
Recently, I noticed that less number of global variables are merged in global-merge pass and in some global variable, array types are used instead of its original type. For example, [4xi8] with align 4 is used for a i32 global variable. For me, it seems that such pattern is observed after r278338 (Resolution-based LTO API).
Are you sure it is performed in the global merge pass? Can you provide an example of input IR where you see this now but didn’t before?
Also can you confirm you’re using the gold-linker?
Can anyone give me little bit more insight if such type change was intended? If then why such uses of array type is needed?
The transformation I know of is the common variables merging, bug it is not new. We use an array because we have to merge globals from different size and an array of bytes is the most straightforward representation for that.
Hi,
Recently, I noticed that less number of global variables are merged in global-merge pass and in some global variable, array types are used instead of its original type. For example, [4xi8] with align 4 is used for a i32 global variable. For me, it seems that such pattern is observed after r278338 (Resolution-based LTO API).
Are you sure it is performed in the global merge pass? Can you provide
an example of input IR where you see this now but didn’t before?
Also can you confirm you’re using the gold-linker?
I used gold linker. In spec2006/perlbench, I observed the less number of globals are merged in GlobalMerge.cpp after r278338. The reason is because, from the very first pass, several global variables use [4xi8] with align 4, instead of its original type i32 after r278338. Current GlobalMerge pass doesn't seem to handle such fancy-aligned globals. If such type change (e.g., from i32 to [4xi8]) in global variables was intended in r278338, I think we should enhance GlobalMerge to handle such cases.
Can you submit a reproduction for Gold please?
We need to understand what changed with the new LTO API.
I compiled below C code for aarch64 in lto using gold (--target=aarch64-linux-gnu -flto -fuse-ld=gold). After r278338, two globals, GVi32_a and GVi32_b, are [4 x i8] type in the input IR to GlobalMerge. Therefore, GlobalMerge do not even start to handle them because as of now it ignores fancy-aligned globals. Before r278338, GVi32_a and GVi32_b seems to be i32 in the input IR to GlobalMerge. Is this change in the input IR expected ?
These are common variables, this is what I mentioned in my first email. Compiling with -fno-commons or defining them with “int GVi32_a = 0;” should solve it.
However r278338 is not supposed to have changed anything on this aspect. I would have expected maybe r279417 playing a role there.
Anyway I don’t have Gold, so I’ll leave Teresa investigate why the change in behavior.
Do you want to try improving global merge to try to handle this case?
Thanks for the test case! I can reproduce this, and see with the
compiler I saved from just before r278338 that this is indeed a chance
in behavior. Looking at why this changed...
Mehdi: I see what is going on:
+ ArrayType *Ty =
+ ArrayType::get(Type::getInt8Ty(RegularLTO.Ctx), I.second.Size);
+ GlobalVariable *OldGV = RegularLTO.CombinedModule->getNamedGlobal(I.first);
+ if (OldGV && OldGV->getType()->getElementType() == Ty) {
+ // Don't create a new global if the type is already correct, just make
+ // sure the alignment is correct.
We compare the existing type to an ArrayType constructed from the
recorded merged common size. So
the original i32 type does not look the same (since it compares
against [4 x i8]).
Not that familiar with type handling in LLVM, but I guess I need to
just check if the type size is the same?
Hi,
Recently, I noticed that less number of global variables are merged in global-merge pass and in some global variable, array types are used instead of its original type. For example, [4xi8] with align 4 is used for a i32 global variable. For me, it seems that such pattern is observed after r278338 (Resolution-based LTO API).
Are you sure it is performed in the global merge pass? Can you provide
an example of input IR where you see this now but didn’t before?
Also can you confirm you’re using the gold-linker?I used gold linker. In spec2006/perlbench, I observed the less number of globals are merged in GlobalMerge.cpp after r278338. The reason is because, from the very first pass, several global variables use [4xi8] with align 4, instead of its original type i32 after r278338. Current GlobalMerge pass doesn't seem to handle such fancy-aligned globals. If such type change (e.g., from i32 to [4xi8]) in global variables was intended in r278338, I think we should enhance GlobalMerge to handle such cases.
Can you submit a reproduction for Gold please?
We need to understand what changed with the new LTO API.I compiled below C code for aarch64 in lto using gold (--target=aarch64-linux-gnu -flto -fuse-ld=gold). After r278338, two globals, GVi32_a and GVi32_b, are [4 x i8] type in the input IR to GlobalMerge. Therefore, GlobalMerge do not even start to handle them because as of now it ignores fancy-aligned globals. Before r278338, GVi32_a and GVi32_b seems to be i32 in the input IR to GlobalMerge. Is this change in the input IR expected ?
-------------------------
int GVi32_a ;
int GVi32_b ;These are common variables, this is what I mentioned in my first
email. Compiling with -fno-commons or defining them with “int GVi32_a
= 0;” should solve it.However r278338 is not supposed to have changed anything on this
aspect. I would have expected maybe r279417 playing a role there.Anyway I don’t have Gold, so I’ll leave Teresa investigate why the
change in behavior.Do you want to try improving global merge to try to handle this case?
—
Mehdi
I’m that familiar with the common variables and how they are generated? If this is pretty common, then I will happy to improve GlboalMerge to handle them. Before I start looking at this I may want to estimate how often this cause GlobalMerge to lose opportunities.
Hi,
Recently, I noticed that less number of global variables are merged in global-merge pass and in some global variable, array types are used instead of its original type. For example, [4xi8] with align 4 is used for a i32 global variable. For me, it seems that such pattern is observed after r278338 (Resolution-based LTO API).
Are you sure it is performed in the global merge pass? Can you provide
an example of input IR where you see this now but didn’t before?
Also can you confirm you’re using the gold-linker?I used gold linker. In spec2006/perlbench, I observed the less number of globals are merged in GlobalMerge.cpp after r278338. The reason is because, from the very first pass, several global variables use [4xi8] with align 4, instead of its original type i32 after r278338. Current GlobalMerge pass doesn’t seem to handle such fancy-aligned globals. If such type change (e.g., from i32 to [4xi8]) in global variables was intended in r278338, I think we should enhance GlobalMerge to handle such cases.
Can you submit a reproduction for Gold please?
We need to understand what changed with the new LTO API.I compiled below C code for aarch64 in lto using gold (–target=aarch64-linux-gnu -flto -fuse-ld=gold). After r278338, two globals, GVi32_a and GVi32_b, are [4 x i8] type in the input IR to GlobalMerge. Therefore, GlobalMerge do not even start to handle them because as of now it ignores fancy-aligned globals. Before r278338, GVi32_a and GVi32_b seems to be i32 in the input IR to GlobalMerge. Is this change in the input IR expected ?
int GVi32_a ;
int GVi32_b ;These are common variables, this is what I mentioned in my first
email. Compiling with -fno-commons or defining them with “int GVi32_a
= 0;” should solve it.
However r278338 is not supposed to have changed anything on this
aspect. I would have expected maybe r279417 playing a role there.
Anyway I don’t have Gold, so I’ll leave Teresa investigate why the
change in behavior.
Do you want to try improving global merge to try to handle this case?
—
MehdiI’m that familiar with the common variables and how they are generated?
Common variables are here because in C (not C++) when you see at the file scope "int a;” you don’t know if a is an external declaration (with a definition in another file) or a definition. While if you write “int a = 0;” then it has to be a definition.
The linker has to merge every “common” (“a” need to have the same address when accessed from different files), but has to adjust the size and the alignment to the maximum of all the commons.
Let me know if it isn’t clear.
If this is pretty common,
They are very “common” 
(Sorry)
then I will happy to improve GlboalMerge to handle them. Before I start looking at this I may want to estimate how often this cause GlobalMerge to lose opportunities.
I was thinking that independently of the common variables, this may enables more opportunities in general.
Right, but that the code I wrote in r279417 right? I guess we should compare the *size* first, and only do something if the size differs.
How was Gold doing it before r278338?
>
> Mehdi: I see what is going on:
>
> + ArrayType *Ty =
> + ArrayType::get(Type::getInt8Ty(RegularLTO.Ctx), I.second.Size);
> + GlobalVariable *OldGV = RegularLTO.CombinedModule->
getNamedGlobal(I.first);
> + if (OldGV && OldGV->getType()->getElementType() == Ty) {
> + // Don't create a new global if the type is already correct, just
make
> + // sure the alignment is correct.
>
> We compare the existing type to an ArrayType constructed from the
> recorded merged common size. So
> the original i32 type does not look the same (since it compares
> against [4 x i8]).Right, but that the code I wrote in r279417 right? I guess we should
compare the *size* first, and only do something if the size differs.
Yeah. In fact the original handling added in r278338 (addCommons) didn't
even compare the size, it always created a new merged common with an
ArrayType.
How was Gold doing it before r278338?
It was just letting the ModuleLinker handle it (which kept the largest one
by size), and then it fixed up the alignment later.
Will change to first compare the size.
I have a fix, just running tests.
FYI: commited in r279911