[Bug tree-optimization/51782] -ftree-sra: Missing address-space information leads to wrong
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51782 --- Comment #22 from Georg-Johann Lay gjl at gcc dot gnu.org 2012-02-21 09:56:08 UTC --- btw, what's the right component for the PR? tree-optimization? middle-end?
[Bug tree-optimization/51782] -ftree-sra: Missing address-space information leads to wrong
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51782 --- Comment #19 from rguenther at suse dot de rguenther at suse dot de 2012-02-20 09:27:48 UTC --- On Fri, 17 Feb 2012, jamborm at gcc dot gnu.org wrote: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51782 --- Comment #17 from Martin Jambor jamborm at gcc dot gnu.org 2012-02-17 17:59:43 UTC --- Created attachment 26695 -- http://gcc.gnu.org/bugzilla/attachment.cgi?id=26695 Untested proposed fix This untested patch fixes the issue for me on a cross-compiler. It would be great if someone who is set up to run the testsuite on a platform with multiple address spaces or a simulator of one could test it a bit. My plan is to discuss this with maintainers next week and if they like the approach, give it a formal bootstrap and test run on x86_64 and submit shortly thereafter if everything goes fine. base returned from get_base_address should never be NULL, so it's safe to assume it isn't. Otherwise the patch looks ok to me. Richard.
[Bug tree-optimization/51782] -ftree-sra: Missing address-space information leads to wrong
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51782 --- Comment #20 from Martin Jambor jamborm at gcc dot gnu.org 2012-02-20 17:27:36 UTC --- (In reply to comment #19) base returned from get_base_address should never be NULL, so it's safe to assume it isn't. Otherwise the patch looks ok to me. Unfortunately, when I was bootstrapping a modified patch without the NULL test on x86_64, it segfaulted. The reason is that expand_debug_expr calls set_mem_attributes_minus_bitpos and in t passes MEM[(struct basic_stringbuf *)__s._M_stringbuf]._M_string which might be OK for debug statements, I don't really know. Even though I guess it might wreck havoc in address spaces in debug info, for now I'm reverting to the original patch from comment #17 for the purposes of this PR. Perhaps get_base_address misses a DECL_P in the condition looking into MEM_REFs?
[Bug tree-optimization/51782] -ftree-sra: Missing address-space information leads to wrong
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51782 --- Comment #21 from rguenther at suse dot de rguenther at suse dot de 2012-02-20 19:44:46 UTC --- On Mon, 20 Feb 2012, jamborm at gcc dot gnu.org wrote: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51782 --- Comment #20 from Martin Jambor jamborm at gcc dot gnu.org 2012-02-20 17:27:36 UTC --- (In reply to comment #19) base returned from get_base_address should never be NULL, so it's safe to assume it isn't. Otherwise the patch looks ok to me. Unfortunately, when I was bootstrapping a modified patch without the NULL test on x86_64, it segfaulted. The reason is that expand_debug_expr calls set_mem_attributes_minus_bitpos and in t passes MEM[(struct basic_stringbuf *)__s._M_stringbuf]._M_string That's an invalid MEM_REF, and the result of set_mem_attributes_minus_bitpos is probably completely bogus. which might be OK for debug statements, I don't really know. Even though I guess it might wreck havoc in address spaces in debug info, for now I'm reverting to the original patch from comment #17 for the purposes of this PR. Ok, probably safer for 4.7 Perhaps get_base_address misses a DECL_P in the condition looking into MEM_REFs? No, sure not - in the above we have a component-ref inside the ADDR_EXPR. Richard.
[Bug tree-optimization/51782] -ftree-sra: Missing address-space information leads to wrong
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51782 --- Comment #18 from Georg-Johann Lay gjl at gcc dot gnu.org 2012-02-19 09:31:52 UTC --- (In reply to comment #17) Created attachment 26695 [details] Untested proposed fix This untested patch fixes the issue for me on a cross-compiler. It would be great if someone who is set up to run the testsuite on a platform with multiple address spaces or a simulator of one could test it a bit. Thanks. It will take some days.
[Bug tree-optimization/51782] -ftree-sra: Missing address-space information leads to wrong
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51782 Martin Jambor jamborm at gcc dot gnu.org changed: What|Removed |Added Status|NEW |ASSIGNED AssignedTo|unassigned at gcc dot |jamborm at gcc dot gnu.org |gnu.org | --- Comment #15 from Martin Jambor jamborm at gcc dot gnu.org 2012-02-17 13:05:02 UTC --- In addition to mistakes corrected by the patch in comment #12, build_ref_for_offset indeed also does not care about address spaces of MEM_REFs it is creating. Thus, mine. Perhaps we should even add a check to a verifier to catch situations when a MEM_REF's zero argument has a TREE_TYPE^2 with a different address space than the MEM_REF itself?
[Bug tree-optimization/51782] -ftree-sra: Missing address-space information leads to wrong
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51782 Martin Jambor jamborm at gcc dot gnu.org changed: What|Removed |Added CC||rguenth at gcc dot gnu.org --- Comment #16 from Martin Jambor jamborm at gcc dot gnu.org 2012-02-17 17:53:34 UTC --- OK, I clearly misremembered where the address space information is supposed to be stored. Richi's patch in comment #12 clearly shows it's the address operand of MEM_REF should carry that information, so please disregard my previous comment. Nevertheless, during expansion of a COMPONENT_REF or any handled_component we do rely on its type's TYPE_ADDR_SPACE because expand_expr_real_1 calls set_mem_attributes with the whole reference tree to deduce the attributes from and it uses the address space of its type. If only the base object or pointer of a reference is supposed to hold the address space information, then this function needs to be changed to look at the base object. I'll attach an untested patch straight away. Alternatively, we could alter types o references built by SRA at each step (in build_ref_for_offset and build_ref_for_model) which also works (I also have a patch) but would be rather inconsistent with how we expand MEM_REFs (but front-end generated stuff looks like this).
[Bug tree-optimization/51782] -ftree-sra: Missing address-space information leads to wrong
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51782 --- Comment #17 from Martin Jambor jamborm at gcc dot gnu.org 2012-02-17 17:59:43 UTC --- Created attachment 26695 -- http://gcc.gnu.org/bugzilla/attachment.cgi?id=26695 Untested proposed fix This untested patch fixes the issue for me on a cross-compiler. It would be great if someone who is set up to run the testsuite on a platform with multiple address spaces or a simulator of one could test it a bit. My plan is to discuss this with maintainers next week and if they like the approach, give it a formal bootstrap and test run on x86_64 and submit shortly thereafter if everything goes fine.
[Bug tree-optimization/51782] -ftree-sra: Missing address-space information leads to wrong
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51782
Georg-Johann Lay gjl at gcc dot gnu.org changed:
What|Removed |Added
Attachment #26262|0 |1
is obsolete||
--- Comment #14 from Georg-Johann Lay gjl at gcc dot gnu.org 2012-01-25
18:25:22 UTC ---
Created attachment 26467
-- http://gcc.gnu.org/bugzilla/attachment.cgi?id=26467
a-bug.c
The draft address space names have been replaced by their final names, which is
__flash now. The draft __pgm is no more supported.
http://gcc.gnu.org/viewcvs?root=gccview=revrev=183529
Updated test case:
==
struct rgb { char r, g, b; };
char read_rgb_ok (const __flash struct rgb *s)
{
return s-r + s-g + s-b;
}
char read_rgb_bug (const __flash struct rgb *s)
{
struct rgb t = *s;
return t.r + t.g + t.b;
}
[Bug tree-optimization/51782] -ftree-sra: Missing address-space information leads to wrong
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51782 --- Comment #13 from Georg-Johann Lay gjl at gcc dot gnu.org 2012-01-18 09:17:29 UTC --- (In reply to comment #12) Author: rguenth Date: Tue Jan 17 14:52:57 2012 New Revision: 183249 Just for feedback: In r183270 the bug is still present (and -fno-tree-sra is work around).
[Bug tree-optimization/51782] -ftree-sra: Missing address-space information leads to wrong
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51782 --- Comment #12 from Richard Guenther rguenth at gcc dot gnu.org 2012-01-17 14:53:06 UTC --- Author: rguenth Date: Tue Jan 17 14:52:57 2012 New Revision: 183249 URL: http://gcc.gnu.org/viewcvs?root=gccview=revrev=183249 Log: 2012-01-17 Richard Guenther rguent...@suse.de PR middle-end/51782 * expr.c (expand_assignment): Take address-space information from the address operand of MEM_REF and TARGET_MEM_REF. (expand_expr_real_1): Likewise. Modified: trunk/gcc/ChangeLog trunk/gcc/expr.c
[Bug tree-optimization/51782] -ftree-sra: Missing address-space information leads to wrong
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51782 Martin Jambor jamborm at gcc dot gnu.org changed: What|Removed |Added CC||jamborm at gcc dot gnu.org --- Comment #10 from Martin Jambor jamborm at gcc dot gnu.org 2012-01-12 17:17:46 UTC --- Where is the address space information about a particular memory access stored in gimple/tree infrastructure? Do you happen to know if this bug started happening recently on the trunk? Thanks.
[Bug tree-optimization/51782] -ftree-sra: Missing address-space information leads to wrong
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51782 --- Comment #11 from Georg-Johann Lay gjl at gcc dot gnu.org 2012-01-12 18:09:13 UTC --- (In reply to comment #10) Where is the address space information about a particular memory access stored in gimple/tree infrastructure? You mean the ADDR_SPACE macros from tree.h?
