| Message ID | 23380910.rol27ql90L@polaris |
|---|---|
| State | New |
| Headers | show |
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
> 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.
On December 7, 2015 5:42:02 PM GMT+01:00, Eric Botcazou <ebotcazou@adacore.com> 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.
On 7 December 2015 at 10:35, Eric Botcazou <ebotcazou@adacore.com> 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 <ebotcazou@adacore.com> > > 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
> 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.
On 8 December 2015 at 10:46, Eric Botcazou <ebotcazou@adacore.com> 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
Index: cfgexpand.c =================================================================== --- cfgexpand.c (revision 231318) +++ cfgexpand.c (working copy) @@ -184,10 +184,15 @@ set_rtl (tree t, rtx x) || SUBREG_P (XEXP (x, 0))) && (REG_P (XEXP (x, 1)) || SUBREG_P (XEXP (x, 1)))) + /* We need to accept PARALLELs for RESUT_DECLs + because of vector types with BLKmode returned + in multiple registers, but they are supposed + to be uncoalesced. */ || (GET_CODE (x) == PARALLEL && SSAVAR (t) && TREE_CODE (SSAVAR (t)) == RESULT_DECL - && !flag_tree_coalesce_vars)) + && (GET_MODE (x) == BLKmode + || !flag_tree_coalesce_vars))) : (MEM_P (x) || x == pc_rtx || (GET_CODE (x) == CONCAT && MEM_P (XEXP (x, 0)) Index: function.c =================================================================== --- function.c (revision 231318) +++ function.c (working copy) @@ -5148,15 +5148,16 @@ expand_function_start (tree subr) /* Compute the return values into a pseudo reg, which we will copy into the true return register after the cleanups are done. */ tree return_type = TREE_TYPE (res); - /* If we may coalesce this result, make sure it has the expected - mode. */ - if (flag_tree_coalesce_vars && is_gimple_reg (res)) - { - tree def = ssa_default_def (cfun, res); - gcc_assert (def); - machine_mode mode = promote_ssa_mode (def, NULL); - set_parm_rtl (res, gen_reg_rtx (mode)); - } + + /* If we may coalesce this result, make sure it has the expected mode + in case it was promoted. But we need not bother about BLKmode. */ + machine_mode promoted_mode + = flag_tree_coalesce_vars && is_gimple_reg (res) + ? promote_ssa_mode (ssa_default_def (cfun, res), NULL) + : BLKmode; + + if (promoted_mode != BLKmode) + set_parm_rtl (res, gen_reg_rtx (promoted_mode)); else if (TYPE_MODE (return_type) != BLKmode && targetm.calls.return_in_msb (return_type)) /* expand_function_end will insert the appropriate padding in