Patchwork [wide-int] Fix comment about GCC_BAD

login
register
mail settings
Submitter Richard Sandiford
Date Nov. 9, 2013, 10:13 a.m.
Message ID <8761s1hoi4.fsf@talisman.default>
Download mbox | patch
Permalink /patch/289961/
State New
Headers show

Comments

Richard Sandiford - Nov. 9, 2013, 10:13 a.m.
There are two instances of:

      if (TREE_CODE (x) != INTEGER_CST)
	GCC_BAD ("invalid constant in %<#pragma pack%> - ignored");
      /* Cannot use tree_to_uhwi here or it will ice if above message printed.  */
      align = tree_to_hwi (x);

But GCC_BAD includes a return statement.

As far as tree_to_hwi vs. tree_to_uhwi goes: tree_to_uhwi will ICE
for big constants, so we can't use it in this context without first
checking tree_fits_uhwi_p and reporting an error if it's false.
Which might be the right thing to do, but is separate from the
wide-int conversion.

The old code used TREE_INT_CST_LOW here, so tree_to_hwi is the direct
equivalent.

Tested on powerpc64-linux-gnu and by rerunning the assembly comparison.
OK to install?

Thanks,
Richard
Kenneth Zadeck - Nov. 9, 2013, 2:27 p.m.
On 11/09/2013 05:13 AM, Richard Sandiford wrote:
> There are two instances of:
>
>        if (TREE_CODE (x) != INTEGER_CST)
> 	GCC_BAD ("invalid constant in %<#pragma pack%> - ignored");
>        /* Cannot use tree_to_uhwi here or it will ice if above message printed.  */
>        align = tree_to_hwi (x);
>
> But GCC_BAD includes a return statement.
>
> As far as tree_to_hwi vs. tree_to_uhwi goes: tree_to_uhwi will ICE
> for big constants, so we can't use it in this context without first
> checking tree_fits_uhwi_p and reporting an error if it's false.
> Which might be the right thing to do, but is separate from the
> wide-int conversion.
>
> The old code used TREE_INT_CST_LOW here, so tree_to_hwi is the direct
> equivalent.
>
> Tested on powerpc64-linux-gnu and by rerunning the assembly comparison.
> OK to install?
>
> Thanks,
> Richard
>
>
> Index: gcc/c-family/c-pragma.c
> ===================================================================
> --- gcc/c-family/c-pragma.c	2013-11-09 09:39:17.797740475 +0000
> +++ gcc/c-family/c-pragma.c	2013-11-09 09:39:27.662807150 +0000
> @@ -151,7 +151,6 @@ handle_pragma_pack (cpp_reader * ARG_UNU
>       {
>         if (TREE_CODE (x) != INTEGER_CST)
>   	GCC_BAD ("invalid constant in %<#pragma pack%> - ignored");
> -      /* Cannot use tree_to_uhwi here or it will ice if above message printed.  */
>         align = tree_to_hwi (x);
>         action = set;
>         if (pragma_lex (&x) != CPP_CLOSE_PAREN)
> @@ -184,7 +183,6 @@ #define GCC_BAD_ACTION do { if (action !
>   	    {
>   	      if (TREE_CODE (x) != INTEGER_CST)
>   		GCC_BAD ("invalid constant in %<#pragma pack%> - ignored");
> -	      /* Cannot use tree_to_uhwi here or it will ice if above message printed.  */
>   	      align = tree_to_hwi (x);
>   	      if (align == -1)
>   		action = set;
this is fine with me, but you should wait for richi to see if he would 
prefer to use tree_to_uhwi.

kenny
Mike Stump - Nov. 9, 2013, 4:35 p.m.
On Nov 9, 2013, at 2:13 AM, Richard Sandiford <rdsandiford@googlemail.com> wrote:
> There are two instances of:
> 
>      if (TREE_CODE (x) != INTEGER_CST)
> 	GCC_BAD ("invalid constant in %<#pragma pack%> - ignored");
>      /* Cannot use tree_to_uhwi here or it will ice if above message printed.  */
>      align = tree_to_hwi (x);

> As far as tree_to_hwi vs. tree_to_uhwi goes: tree_to_uhwi will ICE
> for big constants, so we can't use it in this context without first
> checking tree_fits_uhwi_p and reporting an error if it's false.

> OK to install?

Ok.

Patch

Index: gcc/c-family/c-pragma.c
===================================================================
--- gcc/c-family/c-pragma.c	2013-11-09 09:39:17.797740475 +0000
+++ gcc/c-family/c-pragma.c	2013-11-09 09:39:27.662807150 +0000
@@ -151,7 +151,6 @@  handle_pragma_pack (cpp_reader * ARG_UNU
     {
       if (TREE_CODE (x) != INTEGER_CST)
 	GCC_BAD ("invalid constant in %<#pragma pack%> - ignored");
-      /* Cannot use tree_to_uhwi here or it will ice if above message printed.  */
       align = tree_to_hwi (x);
       action = set;
       if (pragma_lex (&x) != CPP_CLOSE_PAREN)
@@ -184,7 +183,6 @@  #define GCC_BAD_ACTION do { if (action !
 	    {
 	      if (TREE_CODE (x) != INTEGER_CST)
 		GCC_BAD ("invalid constant in %<#pragma pack%> - ignored");
-	      /* Cannot use tree_to_uhwi here or it will ice if above message printed.  */
 	      align = tree_to_hwi (x);
 	      if (align == -1)
 		action = set;