Re: [patch] Fix PR middle-end/68291 & 68292
On 8 December 2015 at 19:48, Eric Botcazou wrote: >> I think that is ok if the testing passes. > > Thanks. It did on the 3 architectures so I have applied the patch. > Thanks, I confirm it also fixes the regressions I noticed on ARM. Christophe. > -- > Eric Botcazou
Re: [patch] Fix PR middle-end/68291 & 68292
> I think that is ok if the testing passes. Thanks. It did on the 3 architectures so I have applied the patch. -- Eric Botcazou
Re: [patch] Fix PR middle-end/68291 & 68292
On 12/08/2015 11:50 AM, Eric Botcazou wrote: I'm going to test it on x86-64, SPARC64 and Aarch64. PR middle-end/68291 PR middle-end/68292 * cfgexpand.c (set_rtl): Always accept mode mismatch for SSA names with BLKmode promoted mode based on RESULT_DECLs. I think that is ok if the testing passes. Bernd
Re: [patch] Fix PR middle-end/68291 & 68292
> Yes you are right. the PASS->FAIL and "PASS disappears" are consequences
> of the new failures above.
OK. The issue is that we used to create a REG:BLK for RESULT_DECL, but now we
get to hard_function_value as originally, which rightfully adjusts it to SI:
val
= targetm.calls.function_value (valtype, func ? func : fntype, outgoing);
if (REG_P (val)
&& GET_MODE (val) == BLKmode)
{
unsigned HOST_WIDE_INT bytes = int_size_in_bytes (valtype);
machine_mode tmpmode;
/* int_size_in_bytes can return -1. We don't need a check here
since the value of bytes will then be large enough that no
mode will match anyway. */
for (tmpmode = GET_CLASS_NARROWEST_MODE (MODE_INT);
tmpmode != VOIDmode;
tmpmode = GET_MODE_WIDER_MODE (tmpmode))
{
/* Have we found a large enough mode? */
if (GET_MODE_SIZE (tmpmode) >= bytes)
break;
}
/* No suitable mode found. */
gcc_assert (tmpmode != VOIDmode);
PUT_MODE (val, tmpmode);
}
and we assert in the second gcc_checking_assert of set_rtl because mode and
promote_ssa_mode don't agree anymore (SI vs BLK). So we need to relax the
second gcc_checking_assert the same way as the first one.
I'm going to test it on x86-64, SPARC64 and Aarch64.
PR middle-end/68291
PR middle-end/68292
* cfgexpand.c (set_rtl): Always accept mode mismatch for SSA names
with BLKmode promoted mode based on RESULT_DECLs.
--
Eric BotcazouIndex: cfgexpand.c
===
--- cfgexpand.c (revision 231394)
+++ cfgexpand.c (working copy)
@@ -203,11 +203,14 @@ set_rtl (tree t, rtx x)
PARM_DECLs and RESULT_DECLs, we'll have been called by
set_parm_rtl, which will give us the default def, so we don't
have to compute it ourselves. For RESULT_DECLs, we accept mode
- mismatches too, as long as we're not coalescing across variables,
- so that we don't reject BLKmode PARALLELs or unpromoted REGs. */
+ mismatches too, as long as we have BLKmode or are not coalescing
+ across variables, so that we don't reject BLKmode PARALLELs or
+ unpromoted REGs. */
gcc_checking_assert (!x || x == pc_rtx || TREE_CODE (t) != SSA_NAME
- || (SSAVAR (t) && TREE_CODE (SSAVAR (t)) == RESULT_DECL
- && !flag_tree_coalesce_vars)
+ || (SSAVAR (t)
+ && TREE_CODE (SSAVAR (t)) == RESULT_DECL
+ && (promote_ssa_mode (t, NULL) == BLKmode
+ || !flag_tree_coalesce_vars))
|| !use_register_for_decl (t)
|| GET_MODE (x) == promote_ssa_mode (t, NULL));
Re: [patch] Fix PR middle-end/68291 & 68292
On 8 December 2015 at 10:46, Eric Botcazou wrote: >> Since you committed this (r231372), I've noticed several regressions >> on ARM and AArch64. >> You can have a look at >> http://people.linaro.org/~christophe.lyon/cross-validation/gcc/trunk/231372/ >> report-build-info.html for more details. > > I presume it's: > > Fail appears [ => FAIL]: > gcc.dg/pr63594-1.c (internal compiler error) > gcc.dg/pr63594-2.c (internal compiler error) > gcc.dg/vect/vect-singleton_1.c (internal compiler error) > gcc.dg/vect/vect-singleton_1.c -flto -ffat-lto-objects (internal compiler > error) > gcc.target/aarch64/aapcs64/func-ret-1.c compilation, -O1 (internal > compiler error) > gcc.target/aarch64/aapcs64/func-ret-1.c compilation, -O2 (internal > compiler error) > gcc.target/aarch64/aapcs64/func-ret-1.c compilation, -O3 -g (internal > compiler error) > gcc.target/aarch64/aapcs64/func-ret-1.c compilation, -Og -g (internal > compiler error) > gcc.target/aarch64/aapcs64/func-ret-1.c compilation, -Os (internal > compiler error) > gcc.target/aarch64/pr65491_1.c (internal compiler error) > > on aarch64? Yes, I'm going to have a look. > Yes you are right. the PASS->FAIL and "PASS disappears" are consequences of the new failures above. You can download the gcc.log files by clicking on the "log" link, it that helps. Thanks, Christophe. > -- > Eric Botcazou
Re: [patch] Fix PR middle-end/68291 & 68292
> Since you committed this (r231372), I've noticed several regressions > on ARM and AArch64. > You can have a look at > http://people.linaro.org/~christophe.lyon/cross-validation/gcc/trunk/231372/ > report-build-info.html for more details. I presume it's: Fail appears [ => FAIL]: gcc.dg/pr63594-1.c (internal compiler error) gcc.dg/pr63594-2.c (internal compiler error) gcc.dg/vect/vect-singleton_1.c (internal compiler error) gcc.dg/vect/vect-singleton_1.c -flto -ffat-lto-objects (internal compiler error) gcc.target/aarch64/aapcs64/func-ret-1.c compilation, -O1 (internal compiler error) gcc.target/aarch64/aapcs64/func-ret-1.c compilation, -O2 (internal compiler error) gcc.target/aarch64/aapcs64/func-ret-1.c compilation, -O3 -g (internal compiler error) gcc.target/aarch64/aapcs64/func-ret-1.c compilation, -Og -g (internal compiler error) gcc.target/aarch64/aapcs64/func-ret-1.c compilation, -Os (internal compiler error) gcc.target/aarch64/pr65491_1.c (internal compiler error) on aarch64? Yes, I'm going to have a look. -- Eric Botcazou
Re: [patch] Fix PR middle-end/68291 & 68292
On 7 December 2015 at 10:35, Eric Botcazou wrote: > Hi, > > it's a couple of regressions in the C testsuite present on SPARC 64-bit and > coming from the new coalescing code which fails to handle vector types with > BLKmode that are returned in multiple registers. The code assigns a BLKmode > REG to the RESULT_DECL of the function in expand_function_start and this later > causes expand_function_end to choke. > > As discussed with Alexandre in the audit trail, the attached minimal fix just > prevents the problematic BLKmode REG from being generated, which appears to be > sufficient to restore the nominal operating mode. > > Tested on x86-64/Linux and SPARC64/Solaris, OK for the mainline? > > > 2015-12-07 Eric Botcazou > > PR middle-end/68291 > PR middle-end/68292 > * cfgexpand.c (set_rtl): Always accept PARALLELs with BLKmode for > SSA names based on RESULT_DECLs. > * function.c (expand_function_start): Do not create BLKmode REGs > for GIMPLE registers when coalescing is enabled. > Hi Eric, Since you committed this (r231372), I've noticed several regressions on ARM and AArch64. You can have a look at http://people.linaro.org/~christophe.lyon/cross-validation/gcc/trunk/231372/report-build-info.html for more details. Can you have a look? Thanks, Christophe. > -- > Eric Botcazou
Re: [patch] Fix PR middle-end/68291 & 68292
On December 7, 2015 5:42:02 PM GMT+01:00, Eric Botcazou wrote: >> Ok. Although thinking about your comment in the PR about not making >such >> vectors gimple registers I wonder what the effects of that would be. > >First of all it's a bit painful to do because is_gimple_reg_type is >defined >inline in gimple-expr.h and adding TYPE_MODE in there causes a >compilation >failure for a bunch of files. More seriously, this can probably be >seen as a >real layering violation so I'm not sure this would be a progress. Yeah, it would also have quite some impact on optimization. Note that if only specific decls are involved they can be made non-registers via DECL_GIMPLE_REG_P. Richard.
Re: [patch] Fix PR middle-end/68291 & 68292
> Ok. Although thinking about your comment in the PR about not making such > vectors gimple registers I wonder what the effects of that would be. First of all it's a bit painful to do because is_gimple_reg_type is defined inline in gimple-expr.h and adding TYPE_MODE in there causes a compilation failure for a bunch of files. More seriously, this can probably be seen as a real layering violation so I'm not sure this would be a progress. -- Eric Botcazou
Re: [patch] Fix PR middle-end/68291 & 68292
On 12/07/2015 10:35 AM, Eric Botcazou wrote: As discussed with Alexandre in the audit trail, the attached minimal fix just prevents the problematic BLKmode REG from being generated, which appears to be sufficient to restore the nominal operating mode. PR middle-end/68291 PR middle-end/68292 * cfgexpand.c (set_rtl): Always accept PARALLELs with BLKmode for SSA names based on RESULT_DECLs. * function.c (expand_function_start): Do not create BLKmode REGs for GIMPLE registers when coalescing is enabled. Ok. Although thinking about your comment in the PR about not making such vectors gimple registers I wonder what the effects of that would be. Bernd
