Re: [patch, arm] Fix PR target/50305 (arm_legitimize_reload_address problem)
On 4 October 2011 16:13, Ulrich Weigand uweig...@de.ibm.com wrote: Ramana Radhakrishnan wrote: On 26 September 2011 15:24, Ulrich Weigand uweig...@de.ibm.com wrote: Is this sufficient, or should I test any other set of options as well? Could you run one set of tests with neon ? Sorry for the delay, but I had to switch to my IGEP board for Neon support, and that's a bit slow ... In any case, I've now completed testing the patch with Neon with no regressions. Just to clarify: in the presence of the other options that are already in dg-options, the test case now fails (with the unpatched compiler) for *any* setting of -mfloat-abi (hard, soft, or softfp). Do you still want me to add a specific setting to the test case? No the mfpu=vfpv3 is fine. OK, thanks. Instead of skipping I was wondering if we could prune the outputs to get this through all the testers we have. Well, the problem is that with certain -march options (e.g. armv7) we get: /home/uweigand/gcc-head/gcc/testsuite/gcc.target/arm/pr50305.c:1:0: error: target CPU does not support ARM mode Ah - ok. Since this is an *error*, pruning the output doesn't really help, the test isn't being run in any case. Otherwise this is OK. Given the above, is the patch now OK as-is? OK by me. Ramana
Re: [patch, arm] Fix PR target/50305 (arm_legitimize_reload_address problem)
On 9 September 2011 18:04, Ulrich Weigand uweig...@de.ibm.com wrote: In theory, LEGITIMIZE_RELOAD_ADDRESS could attempt to handle them by substituting the equivalent constant and then reloading the result. However, this might need additional steps (pushing to the constant pool, reloading the constant pool address, ...) which would lead to significant duplication of code from core reload. This doesn't seem worthwhile at this point ... Thinking about it a bit more after our conversation, we do have the movw / movt instructions for armv7-a - why push this to the constant pool if we can rematerialize it ? Finding a way to use them rather than pushing things out to the constant pool might be an interesting experiment for later .. Could you let me know what configuration this was tested with ( i.e. the arch. flags ? ) and make sure this also ran through a run with the v7-a / softfp /neon configurations ? Regarding the testcase - please add an -mfloat-abi=soft to the testcase so that it tests the specific case that failed in case the default configuration was with softfp. cheers Ramana
Re: [patch, arm] Fix PR target/50305 (arm_legitimize_reload_address problem)
Ramana Radhakrishnan wrote: On 9 September 2011 18:04, Ulrich Weigand uweig...@de.ibm.com wrote: In theory, LEGITIMIZE_RELOAD_ADDRESS could attempt to handle them by substituting the equivalent constant and then reloading the result. However, this might need additional steps (pushing to the constant pool, reloading the constant pool address, ...) which would lead to significant duplication of code from core reload. This doesn't seem worthwhile at this point ... Thinking about it a bit more after our conversation, we do have the movw / movt instructions for armv7-a - why push this to the constant pool if we can rematerialize it ? Finding a way to use them rather than pushing things out to the constant pool might be an interesting experiment for later .. Reload common code will already choose whatever the best option is for reloading a constant, according to target hooks (legitimate_constant_p and preferred_reload_class); it doesn't always push to the constant pool. However, even on ARM there are currently certain cases where pushing to the pool is necessary (floating point; some constants involving symbols). The problem is in this specific case where in an early pass, the back-end LEGITIMIZE_RELOAD_ADDRESS has already handled an address, and in a later pass, one of the registers involved turns out to need a reload from a constant. In this case, reload common code does not get involved any more, it expects the back-end to completely handle it. Now, as I said, the back-end *could* do this, but this would involve basically duplicating the various checks common code does: does this particular constant have to be pushed to the literal pool; if so, does the literal pool address require any further reloads, etc. It seemed to me that this would be a lot of (tricky and hard to test) code being added to the back-end, for only very limited gain since this case should be extremely rare. Thus my patch simply refused to do any back-end specific optimization for addresses involving registers that are equivalent to constants. This still does not mean everything is pushed to the constant pool; it just means that reload will fall back to its default handling if that register is spilled (i.e. checking the target hooks and doing what's required to load the constant). Could you let me know what configuration this was tested with ( i.e. the arch. flags ? ) and make sure this also ran through a run with the v7-a / softfp /neon configurations ? I've bootstrapped with the following config options: $ ../gcc-head/configure --enable-languages=c,c++,fortran --with-arch=armv7-a --with-float=softfp --with-fpu=vfpv3-d16 --with-mode=thumb --prefix=/home/uweigand/gcc-head-install (Which I understand are the Ubuntu system compiler settings.) Is this sufficient, or should I test any other set of options as well? Regarding the testcase - please add an -mfloat-abi=soft to the testcase so that it tests the specific case that failed in case the default configuration was with softfp. Just to clarify: in the presence of the other options that are already in dg-options, the test case now fails (with the unpatched compiler) for *any* setting of -mfloat-abi (hard, soft, or softfp). Do you still want me to add a specific setting to the test case? Thanks, Ulrich -- Dr. Ulrich Weigand GNU Toolchain for Linux on System z and Cell BE ulrich.weig...@de.ibm.com
Re: [patch, arm] Fix PR target/50305 (arm_legitimize_reload_address problem)
On 26 September 2011 15:24, Ulrich Weigand uweig...@de.ibm.com wrote: Ramana Radhakrishnan wrote: On 9 September 2011 18:04, Ulrich Weigand uweig...@de.ibm.com wrote: In theory, LEGITIMIZE_RELOAD_ADDRESS could attempt to handle them by substituting the equivalent constant and then reloading the result. However, this might need additional steps (pushing to the constant pool, reloading the constant pool address, ...) which would lead to significant duplication of code from core reload. This doesn't seem worthwhile at this point ... Thinking about it a bit more after our conversation, we do have the movw / movt instructions for armv7-a - why push this to the constant pool if we can rematerialize it ? Finding a way to use them rather than pushing things out to the constant pool might be an interesting experiment for later .. Reload common code will already choose whatever the best option is for reloading a constant, according to target hooks (legitimate_constant_p and preferred_reload_class); it doesn't always push to the constant pool. However, even on ARM there are currently certain cases where pushing to the pool is necessary (floating point; some constants involving symbols). I see your point. I parsed your last mail as it gets into the constant pool by default. If it does that's a separate problem. Is this sufficient, or should I test any other set of options as well? Could you run one set of tests with neon ? Regarding the testcase - please add an -mfloat-abi=soft to the testcase so that it tests the specific case that failed in case the default configuration was with softfp. Just to clarify: in the presence of the other options that are already in dg-options, the test case now fails (with the unpatched compiler) for *any* setting of -mfloat-abi (hard, soft, or softfp). Do you still want me to add a specific setting to the test case? No the mfpu=vfpv3 is fine. Instead of skipping I was wondering if we could prune the outputs to get this through all the testers we have. Otherwise this is OK. cheers Ramana Thanks, Ulrich -- Dr. Ulrich Weigand GNU Toolchain for Linux on System z and Cell BE ulrich.weig...@de.ibm.com
Re: [patch, arm] Fix PR target/50305 (arm_legitimize_reload_address problem)
On Sat, Sep 10, 2011 at 5:04 AM, Ulrich Weigand uweig...@de.ibm.com wrote: Hello, the problem in PR 50305 turned out to be caused by the ARM back-end LEGITIMIZE_RELOAD_ADDRESS implementation. Interesting the fault goes away with -mfpu=neon, perhaps due to the DI mode operations getting pushed out into NEON registers. You might want to be explicit about the FPU in dg-options. -- Michael
