Re: [gimple-classes, committed 4/6] tree-ssa-tail-merge.c: Use gassign
On Tue, Nov 18, 2014 at 2:59 AM, David Malcolm wrote:
> On Mon, 2014-11-17 at 11:06 +0100, Richard Biener wrote:
>> On Sat, Nov 15, 2014 at 12:00 PM, David Malcolm wrote:
>> > On Thu, 2014-11-13 at 11:45 +0100, Richard Biener wrote:
>> >> On Thu, Nov 13, 2014 at 2:41 AM, David Malcolm
>> >> wrote:
>> >> > On Tue, 2014-11-11 at 11:43 +0100, Richard Biener wrote:
>> >> >> On Tue, Nov 11, 2014 at 8:26 AM, Jakub Jelinek
>> >> >> wrote:
>> >> >> > On Mon, Nov 10, 2014 at 05:27:50PM -0500, David Malcolm wrote:
>> >> >> >> On Sat, 2014-11-08 at 14:56 +0100, Jakub Jelinek wrote:
>> >> >> >> > On Sat, Nov 08, 2014 at 01:07:28PM +0100, Richard Biener wrote:
>> >> >> >> > > To be constructive here - the above case is from within a
>> >> >> >> > > GIMPLE_ASSIGN case label
>> >> >> >> > > and thus I'd have expected
>> >> >> >> > >
>> >> >> >> > > case GIMPLE_ASSIGN:
>> >> >> >> > > {
>> >> >> >> > > gassign *a1 = as_a (s1);
>> >> >> >> > > gassign *a2 = as_a (s2);
>> >> >> >> > > lhs1 = gimple_assign_lhs (a1);
>> >> >> >> > > lhs2 = gimple_assign_lhs (a2);
>> >> >> >> > > if (TREE_CODE (lhs1) != SSA_NAME
>> >> >> >> > > && TREE_CODE (lhs2) != SSA_NAME)
>> >> >> >> > > return (operand_equal_p (lhs1, lhs2, 0)
>> >> >> >> > > && gimple_operand_equal_value_p
>> >> >> >> > > (gimple_assign_rhs1 (a1),
>> >> >> >> > >
>> >> >> >> > > gimple_assign_rhs1 (a2)));
>> >> >> >> > > else if (TREE_CODE (lhs1) == SSA_NAME
>> >> >> >> > >&& TREE_CODE (lhs2) == SSA_NAME)
>> >> >> >> > > return vn_valueize (lhs1) == vn_valueize (lhs2);
>> >> >> >> > > return false;
>> >> >> >> > > }
>> >> >> >> > >
>> >> >> >> > > instead. That's the kind of changes I have expected and have
>> >> >> >> > > approved of.
>> >> >> >> >
>> >> >> >> > But even that looks like just adding extra work for all
>> >> >> >> > developers, with no
>> >> >> >> > gain. You only have to add extra code and extra temporaries, in
>> >> >> >> > switches
>> >> >> >> > typically also have to add {} because of the temporaries and thus
>> >> >> >> > extra
>> >> >> >> > indentation level, and it doesn't simplify anything in the code.
>> >> >> >>
>> >> >> >> The branch attempts to use the C++ typesystem to capture information
>> >> >> >> about the kinds of gimple statement we expect, both:
>> >> >> >> (A) so that the compiler can detect type errors, and
>> >> >> >> (B) as a comprehension aid to the human reader of the code
>> >> >> >>
>> >> >> >> The ideal here is when function params and struct field can be
>> >> >> >> strengthened from "gimple" to a subclass ptr. This captures the
>> >> >> >> knowledge that every use of a function or within a struct has a
>> >> >> >> given
>> >> >> >> gimple code.
>> >> >> >
>> >> >> > I just don't like all the as_a/is_a stuff enforced everywhere,
>> >> >> > it means more typing, more temporaries, more indentation.
>> >> >> > So, as I view it, instead of the checks being done cheaply (yes, I
>> >> >> > think
>> >> >> > the gimple checking as we have right now is very cheap) under the
>> >> >> > hood by the accessors (gimple_assign_{lhs,rhs1} etc.), those changes
>> >> >> > put the burden on the developers, who has to check that manually
>> >> >> > through
>> >> >> > the as_a/is_a stuff everywhere, more typing and uglier syntax.
>> >> >> > I just don't see that as a step forward, instead a huge step
>> >> >> > backwards.
>> >> >> > But perhaps I'm alone with this.
>> >> >> > Can you e.g. compare the size of - lines in your patchset combined,
>> >> >> > and
>> >> >> > size of + lines in your patchset? As in, if your changes lead to
>> >> >> > less
>> >> >> > typing or more.
>> >> >>
>> >> >> I see two ways out here. One is to add overloads to all the functions
>> >> >> taking the special types like
>> >> >>
>> >> >> tree
>> >> >> gimple_assign_rhs1 (gimple *);
>> >> >>
>> >> >> or simply add
>> >> >>
>> >> >> gassign *operator ()(gimple *g) { return as_a (g); }
>> >> >>
>> >> >> into a gimple-compat.h header which you include in places that
>> >> >> are not converted "nicely".
>> >> >
>> >> > Thanks for the suggestions.
>> >> >
>> >> > Am I missing something, or is the gimple-compat.h idea above not valid C
>> >> > ++?
>> >> >
>> >> > Note that "gimple" is still a typedef to
>> >> > gimple_statement_base *
>> >> > (as noted before, the gimple -> gimple * change would break everyone
>> >> > else's patches, so we talked about that as a followup patch for early
>> >> > stage3).
>> >> >
>> >> > Given that, if I try to create an "operator ()" outside of a class, I
>> >> > get this error:
>> >> >
>> >> > ‘gassign* operator()(gimple)’ must be a nonstatic member function
>> >> >
>> >> > which is emitted from cp/decl.c's grok_op_properties:
>> >> > /* An operator function must either be a non-static member
>> >> > function
>> >> > or have at
Re: What is R_X86_64_GOTPLT64 used for?
Hi, On Mon, 17 Nov 2014, H.J. Lu wrote: > It has nothing to do with large model. Yes, I didn't say so. I've used it only to force GCC to emit @GOT relocs (otherwise it would have used @GOTPCREL) to disprove your claim. > The same thing happens to small model. We may be to able optimize it, > independent of GOTPLT. Yes, if we were to optimize this, the difference between GOT and GOTPLT would be very minor. > In any case, -mcmodel=large shouldn't change program behavior. No, it shouldn't of course. Ciao, Michael.
RE: Optimized Allocation of Argument registers
-Original Message- From: Vladimir Makarov [mailto:vmaka...@redhat.com] Sent: Tuesday, November 18, 2014 1:57 AM To: Ajit Kumar Agarwal; gcc Mailing List Cc: Vinod Kathail; Shail Aditya Gupta; Vidhumouli Hunsigida; Nagaraju Mekala Subject: Re: Optimized Allocation of Argument registers On 2014-11-17 8:13 AM, Ajit Kumar Agarwal wrote: > Hello All: > > I was looking at the optimized usage and allocation to argument registers. > There are two aspects to it as follows. > > 1. We need to specify the argument registers as followed by ABI in the target > specific code. Based on the function > argument registers defined in the target dependent code the function > argument registers are passed. If the > number of argument registers defined in the Architecture is large say 6/8 > function argument registers. > Most of the time in the benchmarks we don't pass so many arguments and the > number of arguments passed > is quite less. Since we reserve the function arguments as specified > in the target specific code for the given architecture, leads to unoptimized > usage as this function argument registers will not be used in the function. > Thus we need to steal some of the arguments registers and have the > usage of those in the function depending on the support of the number of > function argument registers. The stealing of function argument registers will > lead more number of registers available that are to be used in the function > and leading to less spill and fetch. > >>The argument registers should be not reserved. They should be present in RTL >>and RA allocator will figure out itself when it can use them. >>That is how other ports work. Thanks Vladimir for Clarifications. > 2. The other aspect of the function argument registers is not spill > and fetch the argument registers as they are live across the function > call. But the liveness is limited to certain point of the called > function after that point the function argument registers are not live > and can be used inside the called function. Other aspect is if there is a > shortage of registers than can the function argument registers should be used > as spill candidate? Will this lead to the optimized code. > > >>You can remove unnecessary code to save/restore arg registers around calls if >>you can figure out that they are not used in called functions. >> There is already code for this written by Tom de Vries. So you can use it. Is the code written by Tom de Vries already a part of trunk? Could you please give the pointer to this part of code. Thanks & Regards Ajit
RE: Optimized Allocation of Argument registers
-Original Message- From: Jeff Law [mailto:l...@redhat.com] Sent: Monday, November 17, 2014 9:27 PM To: Ajit Kumar Agarwal; Vladimir Makarov; gcc Mailing List Cc: Vinod Kathail; Shail Aditya Gupta; Vidhumouli Hunsigida; Nagaraju Mekala Subject: Re: Optimized Allocation of Argument registers On 11/17/14 06:13, Ajit Kumar Agarwal wrote: > Hello All: > > I was looking at the optimized usage and allocation to argument registers. > There are two aspects to it as follows. > > 1. We need to specify the argument registers as followed by ABI in the target > specific code. Based on the function > argument registers defined in the target dependent code the function > argument registers are passed. If the > number of argument registers defined in the Architecture is large say 6/8 > function argument registers. > Most of the time in the benchmarks we don't pass so many arguments and the > number of arguments passed > is quite less. Since we reserve the function arguments as specified > in the target specific code for the given architecture, leads to unoptimized > usage as this function argument registers will not be used in the function. > Thus we need to steal some of the arguments registers and have the > usage of those in the function depending on the support of the number of > function argument registers. The stealing of function argument registers will > lead more number of registers available that are to be used in the function > and leading to less spill and fetch. > > 2. The other aspect of the function argument registers is not spill > and fetch the argument registers as they are live across the function > call. But the liveness is limited to certain point of the called > function after that point the function argument registers are not live > and can be used inside the called function. Other aspect is if there is a > shortage of registers than can the function argument registers should be used > as spill candidate? Will this lead to the optimized code. > > Please let me know what do you think. >>Typically GCC ports do not reserve the function argument/return registers, >>they are allocatable just like any other call clobbered register. >>In essence arguments for function calls are set up at each call site by >>copying values out of pseudo registers, memory, constant initializations, etc >>to the >>appropriate outgoing argument register. The allocator will, when >>possible and profitable try to assign the pseudo register to the appropriate >>argument >>register to eliminate the copies. >>Similarly, GCC copies values out of the incoming argument registers and into >>pseudos at the start of a function. The allocator, again when possible and profitable, will try to allocate the pseudos to the incoming argument >>registers to avoid the copy. >>So in summary, there is no reason to reserve registers for argument passing >>in GCC and doing so would be wasteful. Treat the argument registers as any other call clobbered register and allow the register allocator to make >>appropriate decisions based on liveness, expected uses, call-crossing >>liveness, related >>copies, etc etc. Thanks Jeff for the explanation and Clarifications. Thanks & Regards Ajit jeff
Query about the TREE_TYPE field
I was poking around attribs.c while trial running my tree-type-safety
stuff, and it triggered something in decl_attributes() that seems fishy
to me. It looks like it was part of the fix for
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=35315
decl_attributes() can be passed a tree node which is either decl or a
type, but we get to this little snippet:
if (spec->type_required && DECL_P (*anode))
{
anode = &TREE_TYPE (*anode);
/* Allow ATTR_FLAG_TYPE_IN_PLACE for the type's naming decl. */
if (!(TREE_CODE (*anode) == TYPE_DECL
&& *anode == TYPE_NAME (TYPE_MAIN_VARIANT
(TREE_TYPE (*anode)
flags &= ~(int) ATTR_FLAG_TYPE_IN_PLACE;
}
anode is changed to point to the TREE_TYPE of the original decl, and
*then* checks if it is a TYPE_DECL... That doesnt seem right to me..
we can't have a TYPE_DECL as a TREE_TYPE can we?
is that code suppose to be checking is the original DECL is a TYPE_DECL
rather than the TREE_TYPE?
I added an assert:
anode = &TREE_TYPE (*anode);
+ gcc_assert (TREE_CODE (*anode) != TYPE_DECL);
and it never trips during bootstrap or a full testsuite run. I also
tried the testcase in bugzilla and it compiles fine with the assert in
place...
Maybe the assignment to anode should be after the if instead of in front
of it?
Andrew
Re: Query about the TREE_TYPE field
On 11/18/2014 09:26 AM, Andrew MacLeod wrote:
I was poking around attribs.c while trial running my tree-type-safety
stuff, and it triggered something in decl_attributes() that seems fishy
to me. It looks like it was part of the fix for
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=35315
decl_attributes() can be passed a tree node which is either decl or a
type, but we get to this little snippet:
if (spec->type_required && DECL_P (*anode))
{
anode = &TREE_TYPE (*anode);
/* Allow ATTR_FLAG_TYPE_IN_PLACE for the type's naming decl. */
if (!(TREE_CODE (*anode) == TYPE_DECL
&& *anode == TYPE_NAME (TYPE_MAIN_VARIANT
(TREE_TYPE (*anode)
flags &= ~(int) ATTR_FLAG_TYPE_IN_PLACE;
}
anode is changed to point to the TREE_TYPE of the original decl, and
*then* checks if it is a TYPE_DECL... That doesnt seem right to me..
we can't have a TYPE_DECL as a TREE_TYPE can we?
No.
is that code suppose to be checking is the original DECL is a TYPE_DECL
rather than the TREE_TYPE?
I think so.
Maybe the assignment to anode should be after the if instead of in front
of it?
Probably.
Strange that the 35315 patch fixed the testcase with that change being a
placebo...
Jason
Re: What is R_X86_64_GOTPLT64 used for?
On Tue, Nov 18, 2014 at 5:12 AM, Michael Matz wrote:
> Hi,
>
> On Mon, 17 Nov 2014, H.J. Lu wrote:
>
>> It has nothing to do with large model.
>
> Yes, I didn't say so. I've used it only to force GCC to emit @GOT relocs
> (otherwise it would have used @GOTPCREL) to disprove your claim.
Well, it was just on paper. Linker never implemented such GOTPLT optimization:
[hjl@gnu-6 simple]$ cat main.S
.file "main.c"
.text
.globl _start
.type _start, @function
_start:
.LFB0:
.cfi_startproc
pushq %rbp
.cfi_def_cfa_offset 16
.cfi_offset 6, -16
movq %rsp, %rbp
.cfi_def_cfa_register 6
pushq %rbx
subq $24, %rsp
.L2:
.cfi_offset 3, -24
leaq .L2(%rip), %rbx
movabsq $_GLOBAL_OFFSET_TABLE_-.L2, %r11
addq %r11, %rbx
movabsq $foo@GOTPLT, %rax
movq (%rbx,%rax), %rax
movq %rax, -24(%rbp)
movq -24(%rbp), %rax
call *%rax
movabsq $foo@PLTOFF, %rax
addq %rbx, %rax
call *%rax
addq $24, %rsp
popq %rbx
popq %rbp
.cfi_def_cfa 7, 8
ret
.cfi_endproc
.LFE0:
.size _start, .-_start
.ident "GCC: (GNU) 4.8.3 20140911 (Red Hat 4.8.3-7)"
.section .note.GNU-stack,"",@progbits
[hjl@gnu-6 simple]$ cat foo.c
void
foo (void)
{
}
[hjl@gnu-6 simple]$ make
gcc -fpie -mcmodel=large -c -o main.o main.S
gcc -fpic -c -o foo.o foo.c
./usr/local/bin/ld -shared -o libfoo.so foo.o
./usr/local/bin/ld -o foo main.o libfoo.so
readelf -r main.o
Relocation section '.rela.text' at offset 0x290 contains 3 entries:
Offset Info Type Sym. ValueSym. Name + Addend
0012 0009001d R_X86_64_GOTPC64
_GLOBAL_OFFSET_TABLE_ + 9
001f 000a001e R_X86_64_GOTPLT64 foo + 0
0037 000a001f R_X86_64_PLTOFF64 foo + 0
Relocation section '.rela.eh_frame' at offset 0x2d8 contains 1 entries:
Offset Info Type Sym. ValueSym. Name + Addend
0020 00020002 R_X86_64_PC32 .text + 0
readelf -r foo
Relocation section '.rela.dyn' at offset 0x268 contains 1 entries:
Offset Info Type Sym. ValueSym. Name + Addend
006004b8 00020006 R_X86_64_GLOB_DAT foo + 0
Relocation section '.rela.plt' at offset 0x280 contains 1 entries:
Offset Info Type Sym. ValueSym. Name + Addend
006004d8 00020007 R_X86_64_JUMP_SLO foo + 0
[hjl@gnu-6 simple]$
>> The same thing happens to small model. We may be to able optimize it,
>> independent of GOTPLT.
>
> Yes, if we were to optimize this, the difference between GOT and GOTPLT
> would be very minor.
>
I will give it a thought. But we don't need GOTPLT for it. We should obsolete
GOTPLT.
--
H.J.
Re: Query about the TREE_TYPE field
On 11/18/2014 09:40 AM, Jason Merrill wrote:
On 11/18/2014 09:26 AM, Andrew MacLeod wrote:
I was poking around attribs.c while trial running my tree-type-safety
stuff, and it triggered something in decl_attributes() that seems fishy
to me. It looks like it was part of the fix for
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=35315
decl_attributes() can be passed a tree node which is either decl or a
type, but we get to this little snippet:
if (spec->type_required && DECL_P (*anode))
{
anode = &TREE_TYPE (*anode);
/* Allow ATTR_FLAG_TYPE_IN_PLACE for the type's naming
decl. */
if (!(TREE_CODE (*anode) == TYPE_DECL
&& *anode == TYPE_NAME (TYPE_MAIN_VARIANT
(TREE_TYPE (*anode)
flags &= ~(int) ATTR_FLAG_TYPE_IN_PLACE;
}
anode is changed to point to the TREE_TYPE of the original decl, and
*then* checks if it is a TYPE_DECL... That doesnt seem right to me..
we can't have a TYPE_DECL as a TREE_TYPE can we?
No.
is that code suppose to be checking is the original DECL is a TYPE_DECL
rather than the TREE_TYPE?
I think so.
Maybe the assignment to anode should be after the if instead of in front
of it?
Probably.
Strange that the 35315 patch fixed the testcase with that change being
a placebo...
Jason
Indeed. The condition is negated, so effectively it becomes an
always true condition and the ATTR_FLAG_TYPE_IN_PLACE is always turned
off when a DECL is passed and a type is required..
maybe that works most/all of the time? huh, that also means the code
is the same as it was before the patch :-P
maybe one of the follow up patches in bugzilla supercede it?
Andrew
[CFT, Darwin] libffi merge from upstream
Dominik Vogt and I are trying to get upstream libffi merged back to gcc, so that we can leverage better support for libgo. I've done a heavy handed merge into a branch (likely not the final form), and I'd like it to see wider testing. Especially Darwin, which is known to be broken upstream. But I'm hoping that Darwin can be fixed with a limited amount of ifdefs, rather than the wholesale code duplication that existed previously. Please check out git://gcc.gnu.org/git/gcc.git rth/ffi and --enable-languages=all,go to test libffi usage in both java and go. r~
Re: Query about the TREE_TYPE field
On 11/18/2014 09:52 AM, Andrew MacLeod wrote:
On 11/18/2014 09:40 AM, Jason Merrill wrote:
On 11/18/2014 09:26 AM, Andrew MacLeod wrote:
I was poking around attribs.c while trial running my tree-type-safety
stuff, and it triggered something in decl_attributes() that seems fishy
to me. It looks like it was part of the fix for
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=35315
decl_attributes() can be passed a tree node which is either decl or a
type, but we get to this little snippet:
if (spec->type_required && DECL_P (*anode))
{
anode = &TREE_TYPE (*anode);
/* Allow ATTR_FLAG_TYPE_IN_PLACE for the type's naming
decl. */
if (!(TREE_CODE (*anode) == TYPE_DECL
&& *anode == TYPE_NAME (TYPE_MAIN_VARIANT
(TREE_TYPE (*anode)
flags &= ~(int) ATTR_FLAG_TYPE_IN_PLACE;
}
anode is changed to point to the TREE_TYPE of the original decl, and
*then* checks if it is a TYPE_DECL... That doesnt seem right to me..
we can't have a TYPE_DECL as a TREE_TYPE can we?
No.
is that code suppose to be checking is the original DECL is a TYPE_DECL
rather than the TREE_TYPE?
I think so.
Maybe the assignment to anode should be after the if instead of in
front
of it?
Probably.
Strange that the 35315 patch fixed the testcase with that change
being a placebo...
Jason
Indeed. The condition is negated, so effectively it becomes an
always true condition and the ATTR_FLAG_TYPE_IN_PLACE is always turned
off when a DECL is passed and a type is required..
maybe that works most/all of the time? huh, that also means the code
is the same as it was before the patch :-P
maybe one of the follow up patches in bugzilla supercede it?
Andrew
I tried doing the if before chaning to TREE_TYPE... absolutely no
effect on the testsuite or anything else :-)
What do you think, should I check this in? What is there is clearly
incorrect.we could also revert the original patch since that is what
the code base is effectively is doing at the moment...
Andrew
* attribs.c (decl_attributes): Check for TYPE_DECL before switching to
using the TREE_TYPE.
Index: attribs.c
===
*** attribs.c (revision 217234)
--- attribs.c (working copy)
*** decl_attributes (tree *node, tree attrib
*** 501,512
the decl's type in place here. */
if (spec->type_required && DECL_P (*anode))
{
- anode = &TREE_TYPE (*anode);
/* Allow ATTR_FLAG_TYPE_IN_PLACE for the type's naming decl. */
if (!(TREE_CODE (*anode) == TYPE_DECL
&& *anode == TYPE_NAME (TYPE_MAIN_VARIANT
(TREE_TYPE (*anode)
flags &= ~(int) ATTR_FLAG_TYPE_IN_PLACE;
}
if (spec->function_type_required && TREE_CODE (*anode) != FUNCTION_TYPE
--- 501,512
the decl's type in place here. */
if (spec->type_required && DECL_P (*anode))
{
/* Allow ATTR_FLAG_TYPE_IN_PLACE for the type's naming decl. */
if (!(TREE_CODE (*anode) == TYPE_DECL
&& *anode == TYPE_NAME (TYPE_MAIN_VARIANT
(TREE_TYPE (*anode)
flags &= ~(int) ATTR_FLAG_TYPE_IN_PLACE;
+ anode = &TREE_TYPE (*anode);
}
if (spec->function_type_required && TREE_CODE (*anode) != FUNCTION_TYPE
RFC: Move .plt after .text in x86-64 binaries
Hi, Currently x86-64 linker generate binaries with branch overflow for large text section size: https://sourceware.org/bugzilla/show_bug.cgi?id=17592 It happens to small, medium and large models. I will update linker to check for branch overflow. In the meantime, Michael suggested http://www.sourceware.org/ml/binutils/2006-03/msg00276.html place .plt after .text so that PLT is between text and GOT: text PLT readonly data GOT instead of PLT text readonly data GOT I implemented it on hjl/plt branch. Any comments? Roland, will it be a problem for NaCL? Thanks. -- H.J.
Re: Query about the TREE_TYPE field
On 11/18/14 09:30, Andrew MacLeod wrote: I tried doing the if before chaning to TREE_TYPE... absolutely no effect on the testsuite or anything else :-) What do you think, should I check this in? What is there is clearly incorrect.we could also revert the original patch since that is what the code base is effectively is doing at the moment... What I tend to do in these situations is roll back to the version prior to the "fix" and try the testcase with that compiler. Then I can walk through under GDB control to find out whatever I need. Clearly something needs to change here, but the question is what :-) And I think rolling back and debugging that compiler is the best way to know get more information to allow this to move forward. Jeff
Re: RFC: Move .plt after .text in x86-64 binaries
The relative order of multiple executable sections inside the executable segment (distinct from nonexecutable read-only segment) is not important to NaCl.
Re: Query about the TREE_TYPE field
On 11/18/2014 01:36 PM, Jeff Law wrote:
On 11/18/14 09:30, Andrew MacLeod wrote:
I tried doing the if before chaning to TREE_TYPE... absolutely no
effect on the testsuite or anything else :-)
What do you think, should I check this in? What is there is clearly
incorrect.we could also revert the original patch since that is what
the code base is effectively is doing at the moment...
What I tend to do in these situations is roll back to the version
prior to the "fix" and try the testcase with that compiler. Then I
can walk through under GDB control to find out whatever I need.
Clearly something needs to change here, but the question is what :-)
And I think rolling back and debugging that compiler is the best way
to know get more information to allow this to move forward.
From 2008. gah. anyway, Doesn't help. The code snippet never
triggers. The function is called 240 times on the testcase, and each
time its a FUNCTION_DECL, which becomes a FUNCTION_TYPE when we take
TREE_TYPE.
Test testcase ICE's before and after the change.
So it effectively does nothing. Unless Jason can think of a good
reason for it, we probably ought to turf it. Its effectively a NOP.
Andrew
* attribs.c: Remove always true condition, as TREE_TYPE(x) will never
compare equal to a TYPE_DECL.
Index: attribs.c
===
*** attribs.c (revision 217234)
--- attribs.c (working copy)
*** decl_attributes (tree *node, tree attrib
*** 502,512
if (spec->type_required && DECL_P (*anode))
{
anode = &TREE_TYPE (*anode);
! /* Allow ATTR_FLAG_TYPE_IN_PLACE for the type's naming decl. */
! if (!(TREE_CODE (*anode) == TYPE_DECL
! && *anode == TYPE_NAME (TYPE_MAIN_VARIANT
! (TREE_TYPE (*anode)
! flags &= ~(int) ATTR_FLAG_TYPE_IN_PLACE;
}
if (spec->function_type_required && TREE_CODE (*anode) != FUNCTION_TYPE
--- 502,508
if (spec->type_required && DECL_P (*anode))
{
anode = &TREE_TYPE (*anode);
! flags &= ~(int) ATTR_FLAG_TYPE_IN_PLACE;
}
if (spec->function_type_required && TREE_CODE (*anode) != FUNCTION_TYPE
Re: RFC: Move .plt after .text in x86-64 binaries
One issue with text PLT readonly data GOT layout is it uses smaller data size for larger text size since the layout with data is text PLT readonly data GOT data The data size is reduced by PLT size. If PLT is very large, impact may be visible. -- H.J.
Re: Query about the TREE_TYPE field
On 11/18/2014 02:32 PM, Andrew MacLeod wrote: So it effectively does nothing. Unless Jason can think of a good reason for it, we probably ought to turf it. Its effectively a NOP. Yeah, go ahead. Jason
Re: [gimple-classes, committed 4/6] tree-ssa-tail-merge.c: Use gassign
On Tue, 2014-11-18 at 10:53 +0100, Richard Biener wrote:
> On Tue, Nov 18, 2014 at 2:59 AM, David Malcolm wrote:
> > On Mon, 2014-11-17 at 11:06 +0100, Richard Biener wrote:
> >> On Sat, Nov 15, 2014 at 12:00 PM, David Malcolm
> >> wrote:
> >> > On Thu, 2014-11-13 at 11:45 +0100, Richard Biener wrote:
> >> >> On Thu, Nov 13, 2014 at 2:41 AM, David Malcolm
> >> >> wrote:
> >> >> > On Tue, 2014-11-11 at 11:43 +0100, Richard Biener wrote:
> >> >> >> On Tue, Nov 11, 2014 at 8:26 AM, Jakub Jelinek
> >> >> >> wrote:
> >> >> >> > On Mon, Nov 10, 2014 at 05:27:50PM -0500, David Malcolm wrote:
> >> >> >> >> On Sat, 2014-11-08 at 14:56 +0100, Jakub Jelinek wrote:
> >> >> >> >> > On Sat, Nov 08, 2014 at 01:07:28PM +0100, Richard Biener wrote:
> >> >> >> >> > > To be constructive here - the above case is from within a
> >> >> >> >> > > GIMPLE_ASSIGN case label
> >> >> >> >> > > and thus I'd have expected
> >> >> >> >> > >
> >> >> >> >> > > case GIMPLE_ASSIGN:
> >> >> >> >> > > {
> >> >> >> >> > > gassign *a1 = as_a (s1);
> >> >> >> >> > > gassign *a2 = as_a (s2);
> >> >> >> >> > > lhs1 = gimple_assign_lhs (a1);
> >> >> >> >> > > lhs2 = gimple_assign_lhs (a2);
> >> >> >> >> > > if (TREE_CODE (lhs1) != SSA_NAME
> >> >> >> >> > > && TREE_CODE (lhs2) != SSA_NAME)
> >> >> >> >> > > return (operand_equal_p (lhs1, lhs2, 0)
> >> >> >> >> > > && gimple_operand_equal_value_p
> >> >> >> >> > > (gimple_assign_rhs1 (a1),
> >> >> >> >> > >
> >> >> >> >> > > gimple_assign_rhs1 (a2)));
> >> >> >> >> > > else if (TREE_CODE (lhs1) == SSA_NAME
> >> >> >> >> > >&& TREE_CODE (lhs2) == SSA_NAME)
> >> >> >> >> > > return vn_valueize (lhs1) == vn_valueize (lhs2);
> >> >> >> >> > > return false;
> >> >> >> >> > > }
> >> >> >> >> > >
> >> >> >> >> > > instead. That's the kind of changes I have expected and have
> >> >> >> >> > > approved of.
> >> >> >> >> >
> >> >> >> >> > But even that looks like just adding extra work for all
> >> >> >> >> > developers, with no
> >> >> >> >> > gain. You only have to add extra code and extra temporaries,
> >> >> >> >> > in switches
> >> >> >> >> > typically also have to add {} because of the temporaries and
> >> >> >> >> > thus extra
> >> >> >> >> > indentation level, and it doesn't simplify anything in the code.
> >> >> >> >>
> >> >> >> >> The branch attempts to use the C++ typesystem to capture
> >> >> >> >> information
> >> >> >> >> about the kinds of gimple statement we expect, both:
> >> >> >> >> (A) so that the compiler can detect type errors, and
> >> >> >> >> (B) as a comprehension aid to the human reader of the code
> >> >> >> >>
> >> >> >> >> The ideal here is when function params and struct field can be
> >> >> >> >> strengthened from "gimple" to a subclass ptr. This captures the
> >> >> >> >> knowledge that every use of a function or within a struct has a
> >> >> >> >> given
> >> >> >> >> gimple code.
> >> >> >> >
> >> >> >> > I just don't like all the as_a/is_a stuff enforced everywhere,
> >> >> >> > it means more typing, more temporaries, more indentation.
> >> >> >> > So, as I view it, instead of the checks being done cheaply (yes, I
> >> >> >> > think
> >> >> >> > the gimple checking as we have right now is very cheap) under the
> >> >> >> > hood by the accessors (gimple_assign_{lhs,rhs1} etc.), those
> >> >> >> > changes
> >> >> >> > put the burden on the developers, who has to check that manually
> >> >> >> > through
> >> >> >> > the as_a/is_a stuff everywhere, more typing and uglier syntax.
> >> >> >> > I just don't see that as a step forward, instead a huge step
> >> >> >> > backwards.
> >> >> >> > But perhaps I'm alone with this.
> >> >> >> > Can you e.g. compare the size of - lines in your patchset
> >> >> >> > combined, and
> >> >> >> > size of + lines in your patchset? As in, if your changes lead to
> >> >> >> > less
> >> >> >> > typing or more.
> >> >> >>
> >> >> >> I see two ways out here. One is to add overloads to all the
> >> >> >> functions
> >> >> >> taking the special types like
> >> >> >>
> >> >> >> tree
> >> >> >> gimple_assign_rhs1 (gimple *);
> >> >> >>
> >> >> >> or simply add
> >> >> >>
> >> >> >> gassign *operator ()(gimple *g) { return as_a (g); }
> >> >> >>
> >> >> >> into a gimple-compat.h header which you include in places that
> >> >> >> are not converted "nicely".
> >> >> >
> >> >> > Thanks for the suggestions.
> >> >> >
> >> >> > Am I missing something, or is the gimple-compat.h idea above not
> >> >> > valid C
> >> >> > ++?
> >> >> >
> >> >> > Note that "gimple" is still a typedef to
> >> >> > gimple_statement_base *
> >> >> > (as noted before, the gimple -> gimple * change would break everyone
> >> >> > else's patches, so we talked about that as a followup patch for early
> >> >> > stage3).
> >> >> >
> >> >> > Given that, if I try to create an "operator ()" ou
gcc-4_9 inlines less funcs than gcc-4_8 because of used_as_abstract_origin flag.
We see an inline problem as below caused by r201408
(https://gcc.gnu.org/ml/gcc-patches/2013-08/msg00027.html).
hoo() {
foo();
...
}
foo {
goo();
...
}
foo is func splitted, so its body changes to
foo {
goo();
...
foo.part();
}
and the used_as_abstract_origin of cgraph node of foo will be set to
true after func splitting.
In ipa-inline, when inlining foo into hoo, the original node of foo
will not be reused as clone node because used_as_abstract_origin of
cgraph node of foo is true and can_remove_node_now_p_1 will return
false, so that a new clone node of foo will be created. This is the
case in gcc-4_9.
In gcc-4_8, the original node of foo will be reused as clone node.
gcc-4_8
foo
|
goo
gcc-4_9
foofoo_clone
\/
goo
Because of the difference of whether to create a new clone for foo,
when inlining goo to foo, the overall growth of inlining all callsites
of goo in gcc-4_8 will be less than gcc-4_9 (goo has two callsites in
gcc-4_9 but only one in gcc-4_8). If we have many cases like this,
gcc-4_8 will actually have more inline growth budget than gcc-4_9 and
will inline more aggressively than gcc-4_9.
I don't understand the exact usage of the check about
node->used_as_abstract_origin in can_remove_node_now_p_1, but I feel
puzzled about following two points:
1. https://gcc.gnu.org/ml/gcc-patches/2013-08/msg00027.html said the
patch was to ensure all abstract origin functions do have nodes
attached. However, even if the node of origin function is reused as a
clone node, a new clone node will be created in following code in
symbol_table::remove_unreachable_nodes if only the node that needs
abstract origin is reachable.
if (TREE_CODE (node->decl) == FUNCTION_DECL
&& DECL_ABSTRACT_ORIGIN (node->decl))
{
struct cgraph_node *origin_node
= cgraph_node::get_create (DECL_ABSTRACT_ORIGIN (node->decl));
origin_node->used_as_abstract_origin = true;
enqueue_node (origin_node, &first, &reachable);
}
2. DECL_ABSTRACT_ORIGIN(decl) seems only useful for debug info of
clone nodes. But now the check of used_as_abstract_origin affect
inline decisions, which should be the same with or without keeping
debug info.
Thanks,
Wei.
Re: gcc-4_9 inlines less funcs than gcc-4_8 because of used_as_abstract_origin flag.
On November 19, 2014 8:13:09 AM CET, Wei Mi wrote:
>We see an inline problem as below caused by r201408
>(https://gcc.gnu.org/ml/gcc-patches/2013-08/msg00027.html).
>
>hoo() {
> foo();
> ...
>}
>
>foo {
> goo();
> ...
>}
>
>foo is func splitted, so its body changes to
>
>foo {
> goo();
> ...
> foo.part();
>}
>
>and the used_as_abstract_origin of cgraph node of foo will be set to
>true after func splitting.
>
>In ipa-inline, when inlining foo into hoo, the original node of foo
>will not be reused as clone node because used_as_abstract_origin of
>cgraph node of foo is true and can_remove_node_now_p_1 will return
>false, so that a new clone node of foo will be created. This is the
>case in gcc-4_9.
>In gcc-4_8, the original node of foo will be reused as clone node.
>
>gcc-4_8
>foo
> |
>goo
>
>gcc-4_9
>foofoo_clone
>\/
> goo
>
>Because of the difference of whether to create a new clone for foo,
>when inlining goo to foo, the overall growth of inlining all callsites
>of goo in gcc-4_8 will be less than gcc-4_9 (goo has two callsites in
>gcc-4_9 but only one in gcc-4_8). If we have many cases like this,
>gcc-4_8 will actually have more inline growth budget than gcc-4_9 and
>will inline more aggressively than gcc-4_9.
>
>I don't understand the exact usage of the check about
>node->used_as_abstract_origin in can_remove_node_now_p_1, but I feel
>puzzled about following two points:
>
>1. https://gcc.gnu.org/ml/gcc-patches/2013-08/msg00027.html said the
>patch was to ensure all abstract origin functions do have nodes
>attached. However, even if the node of origin function is reused as a
>clone node, a new clone node will be created in following code in
>symbol_table::remove_unreachable_nodes if only the node that needs
>abstract origin is reachable.
>
> if (TREE_CODE (node->decl) == FUNCTION_DECL
> && DECL_ABSTRACT_ORIGIN (node->decl))
>{
> struct cgraph_node *origin_node
> = cgraph_node::get_create (DECL_ABSTRACT_ORIGIN (node->decl));
> origin_node->used_as_abstract_origin = true;
> enqueue_node (origin_node, &first, &reachable);
>}
>
>2. DECL_ABSTRACT_ORIGIN(decl) seems only useful for debug info of
>clone nodes. But now the check of used_as_abstract_origin affect
>inline decisions, which should be the same with or without keeping
>debug info.
I think we need to keep the functions but do not need to account for them in
the unit size if we otherwise could remove them
Richard.
>Thanks,
>Wei.
