PR63676, exit tree fold when node be TREE_CLOBBER_P
diff mbox

Message ID CAFiYyc0hnaRhCU5gq+ZnrEyCNxsf7N7hHf=CGkhV6EuJhkQMoA@mail.gmail.com
State New
Headers show

Commit Message

Richard Biener Nov. 7, 2014, 10:35 a.m. UTC
On Fri, Nov 7, 2014 at 11:22 AM, Jiong Wang <jiong.wang@arm.com> wrote:
> the problem is caused by constant fold of node with TREE_CLOBBER_P be true.
>
> according to rtl expander, the purpose of clobber is to mark the going out
> of scope.
>
>             if (TREE_CLOBBER_P (rhs))
>               /* This is a clobber to mark the going out of scope for
>                  this LHS.  */
>
> for vshuf-v16hi, there will be such node
>
>   <bb 5>:
>   r ={v} {CLOBBER};
>
> while the new added "fold_all_stmts" since r216728 will invoke generic
> "fold"
> and that function in fold-const.c has a bug when folding CONSTRUCTOR. we
> should not do
> fold if the tree node is also with TREE_THIS_VOLATILE (t) be true, otherwise
> we will
> generate extra insn during expand.
>
> for example, above assignment will be transformed into
>
>   r = { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 };
>
> while OImode immediate move is not supported when "-mcpu=cortex-a9
> -mfloat-abi=softfp -mfpu=neon" specified,
> thus trigger "insn_invalid_p" error for this testcase.
>
> bootstrap ok on x86-64, no regression.
> ICE on arm gone away.
>
> ok to trunk?

Please instead guard the GIMPLE_SINGLE_RHS case in fold_gimple_assign
instead, like

ok with that change.  If you like you can guard fold () as well, but please
inside the case CONSTRUCTOR: case only.

Thanks,
Richard.

> gcc/
>   PR tree/63676
>   fold-const.c (fold): Do not fold node when TREE_CLOBBER_P be true.

Comments

tejas belagod Nov. 7, 2014, 10:57 a.m. UTC | #1
On 07/11/14 10:35, Richard Biener wrote:
> On Fri, Nov 7, 2014 at 11:22 AM, Jiong Wang <jiong.wang@arm.com> wrote:
>> the problem is caused by constant fold of node with TREE_CLOBBER_P be true.
>>
>> according to rtl expander, the purpose of clobber is to mark the going out
>> of scope.
>>
>>              if (TREE_CLOBBER_P (rhs))
>>                /* This is a clobber to mark the going out of scope for
>>                   this LHS.  */
>>
>> for vshuf-v16hi, there will be such node
>>
>>    <bb 5>:
>>    r ={v} {CLOBBER};
>>
>> while the new added "fold_all_stmts" since r216728 will invoke generic
>> "fold"
>> and that function in fold-const.c has a bug when folding CONSTRUCTOR. we
>> should not do
>> fold if the tree node is also with TREE_THIS_VOLATILE (t) be true, otherwise
>> we will
>> generate extra insn during expand.
>>
>> for example, above assignment will be transformed into
>>
>>    r = { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 };
>>
>> while OImode immediate move is not supported when "-mcpu=cortex-a9
>> -mfloat-abi=softfp -mfpu=neon" specified,
>> thus trigger "insn_invalid_p" error for this testcase.
>>
>> bootstrap ok on x86-64, no regression.
>> ICE on arm gone away.
>>
>> ok to trunk?
>
> Please instead guard the GIMPLE_SINGLE_RHS case in fold_gimple_assign
> instead, like
>
> Index: gcc/gimple-fold.c
> ===================================================================
> --- gcc/gimple-fold.c   (revision 217213)
> +++ gcc/gimple-fold.c   (working copy)
> @@ -320,6 +320,9 @@
>         {
>           tree rhs = gimple_assign_rhs1 (stmt);
>
> +       if (TREE_CLOBBER_P (rhs))
> +         return NULL_TREE;
> +
>          if (REFERENCE_CLASS_P (rhs))
>            return maybe_fold_reference (rhs, false);
>
> ok with that change.  If you like you can guard fold () as well, but please
> inside the case CONSTRUCTOR: case only.

But TREE_CLOBBER_P () checks for CONSTRUCTOR anyway. Does it have a 
chance of being folded between the start of fold () and the  case 
CONSTRUCTOR: given its a CLOBBER? What am I missing?

Thanks,
Tejas.

Patch
diff mbox

Index: gcc/gimple-fold.c
===================================================================
--- gcc/gimple-fold.c   (revision 217213)
+++ gcc/gimple-fold.c   (working copy)
@@ -320,6 +320,9 @@ 
       {
         tree rhs = gimple_assign_rhs1 (stmt);

+       if (TREE_CLOBBER_P (rhs))
+         return NULL_TREE;
+
        if (REFERENCE_CLASS_P (rhs))
          return maybe_fold_reference (rhs, false);