diff mbox

force_const_mem VOIDmode

Message ID 20130607020053.GJ6878@bubble.grove.modra.org
State New
Headers show

Commit Message

Alan Modra June 7, 2013, 2 a.m. UTC
force_const_mem() isn't supposed to handle VOIDmode or BLKmode, so the
check for VOIDmode when aligning is needless.  If we ever did get one
of these modes in a constant pool, this

  pool->offset += GET_MODE_SIZE (mode);

won't add to the pool size, and output_constant_pool_2() will hit a
gcc_unreachable().  Bootstrapped etc. powerpc64-linux.

	* varasm.c (force_const_mem): Assert mode is not VOID or BLK.

Comments

Richard Biener June 7, 2013, 8:47 a.m. UTC | #1
On Fri, Jun 7, 2013 at 4:00 AM, Alan Modra <amodra@gmail.com> wrote:
> force_const_mem() isn't supposed to handle VOIDmode or BLKmode, so the
> check for VOIDmode when aligning is needless.  If we ever did get one
> of these modes in a constant pool, this
>
>   pool->offset += GET_MODE_SIZE (mode);
>
> won't add to the pool size, and output_constant_pool_2() will hit a
> gcc_unreachable().  Bootstrapped etc. powerpc64-linux.

Ok.

Thanks,
Richard.

>         * varasm.c (force_const_mem): Assert mode is not VOID or BLK.
>
> Index: gcc/varasm.c
> ===================================================================
> --- gcc/varasm.c        (revision 199718)
> +++ gcc/varasm.c        (working copy)
> @@ -3567,7 +3575,8 @@
>    *slot = desc;
>
>    /* Align the location counter as required by EXP's data type.  */
> -  align = GET_MODE_ALIGNMENT (mode == VOIDmode ? word_mode : mode);
> +  gcc_checking_assert (mode != VOIDmode && mode != BLKmode);
> +  align = GET_MODE_ALIGNMENT (mode);
>  #ifdef CONSTANT_ALIGNMENT
>    {
>      tree type = lang_hooks.types.type_for_mode (mode, 0);
>
> --
> Alan Modra
> Australia Development Lab, IBM
Andreas Krebbel June 13, 2013, 2:29 p.m. UTC | #2
On 07/06/13 04:00, Alan Modra wrote:
> force_const_mem() isn't supposed to handle VOIDmode or BLKmode, so the
> check for VOIDmode when aligning is needless.  If we ever did get one
> of these modes in a constant pool, this
> 
>   pool->offset += GET_MODE_SIZE (mode);

Hi Alan,

the assertion is triggered on S/390 during bootstrap.

The S/390 movmem_short expander emits the following insn:

(insn 793 3147 795 93 (parallel [
            (set (mem:BLK (reg/f:SI 509 [ gi_filename ]) [0 A8])
                (mem:BLK (reg/f:SI 510 [ gcov_prefix ]) [0 A8]))
            (use (reg:SI 511 [ prefix_length ]))
            (use (const:BLK (unspec:BLK [
                            (const_int 0 [0])
                        ] UNSPEC_INSN)))
            (clobber (reg:SI 1288))
        ])

For machines without larl (load address relative long) the RTX:

(const:BLK (unspec:BLK [
                            (const_int 0 [0])
                        ] UNSPEC_INSN))

is supposed to be pushed into the literal pool. The expression represents the target of the execute
instruction. It doesn't matter that the literal pool offset calculation doesn't work with this RTX
the backend does this in machine dependent reorg anyway on its own.

Picking a different mode wouldn't be correct for our purposes since the size depends on the pattern
containing the expression.

So from a S/390 perspective I think the !BLKmode assertion should be removed.

Bye,

-Andreas-


> 
> won't add to the pool size, and output_constant_pool_2() will hit a
> gcc_unreachable().  Bootstrapped etc. powerpc64-linux.
> 
> 	* varasm.c (force_const_mem): Assert mode is not VOID or BLK.
> 
> Index: gcc/varasm.c
> ===================================================================
> --- gcc/varasm.c	(revision 199718)
> +++ gcc/varasm.c	(working copy)
> @@ -3567,7 +3575,8 @@
>    *slot = desc;
> 
>    /* Align the location counter as required by EXP's data type.  */
> -  align = GET_MODE_ALIGNMENT (mode == VOIDmode ? word_mode : mode);
> +  gcc_checking_assert (mode != VOIDmode && mode != BLKmode);
> +  align = GET_MODE_ALIGNMENT (mode);
>  #ifdef CONSTANT_ALIGNMENT
>    {
>      tree type = lang_hooks.types.type_for_mode (mode, 0);
>
Alan Modra June 13, 2013, 3:31 p.m. UTC | #3
On Thu, Jun 13, 2013 at 04:29:44PM +0200, Andreas Krebbel wrote:
> On 07/06/13 04:00, Alan Modra wrote:
> > force_const_mem() isn't supposed to handle VOIDmode or BLKmode, so the
> > check for VOIDmode when aligning is needless.  If we ever did get one
> > of these modes in a constant pool, this
> > 
> >   pool->offset += GET_MODE_SIZE (mode);
> 
> Hi Alan,
> 
> the assertion is triggered on S/390 during bootstrap.

Reverting the patch seems the simplest thing to do, so I've done that.
Apologies for the breakage.
diff mbox

Patch

Index: gcc/varasm.c
===================================================================
--- gcc/varasm.c	(revision 199718)
+++ gcc/varasm.c	(working copy)
@@ -3567,7 +3575,8 @@ 
   *slot = desc;
 
   /* Align the location counter as required by EXP's data type.  */
-  align = GET_MODE_ALIGNMENT (mode == VOIDmode ? word_mode : mode);
+  gcc_checking_assert (mode != VOIDmode && mode != BLKmode);
+  align = GET_MODE_ALIGNMENT (mode);
 #ifdef CONSTANT_ALIGNMENT
   {
     tree type = lang_hooks.types.type_for_mode (mode, 0);