On 04/08/2013 03:45 AM, Richard Biener wrote:
>> @@ -8584,6 +8584,43 @@ simplify_cond_using_ranges (gimple stmt)>> }>> }>>>> + /* If we have a comparison of a SSA_NAME boolean against>> + a constant (which obviously must be [0..1]). See if the>> + SSA_NAME was set by a type conversion where the source>> + of the conversion is another SSA_NAME with a range [0..1].>> +>> + If so, we can replace the SSA_NAME in the comparison with>> + the RHS of the conversion. This will often make the type>> + conversion dead code which DCE will clean up. */>> + if (TREE_CODE (op0) == SSA_NAME>> + && TREE_CODE (TREE_TYPE (op0)) == BOOLEAN_TYPE>> Use>> (TREE_CODE (TREE_TYPE (op0)) == BOOLEAN_TYPE> || (INTEGRAL_TYPE_P (TREE_TYPE (op0))> && TYPE_PRECISION (TREE_TYPE (op0)) == 1))>> to catch some more cases.
Good catch. Done.
>>> + && is_gimple_min_invariant (op1))>> In this case it's simpler to test TREE_CODE (op1) == INTEGER_CST.
Agreed & fixed.
>>> + {>> + gimple def_stmt = SSA_NAME_DEF_STMT (op0);>> + tree innerop;>> +>> + if (!is_gimple_assign (def_stmt)>> + || !CONVERT_EXPR_CODE_P (gimple_assign_rhs_code (def_stmt)))>> + return false;>> +>> + innerop = gimple_assign_rhs1 (def_stmt);>> +>> + if (!SSA_NAME_OCCURS_IN_ABNORMAL_PHI (innerop))>> As Steven said, the abnormal check is not necessary, but for completeness> you should check TREE_CODE (innerop) == SSA_NAME. Valid (but> unfolded) GIMPLE can have (_Bool) 1, too.
Agreed & fixed.
>> Note that we already have code with similar functionality (see if a> conversion would alter the value of X) as part of optimizing> (T1)(T2)X to (T1)X in simplify_conversion_using_ranges. Maybe> a part of it can be split out and used to simplify conditions for> a bigger range of types than just compares against boolean 0/1.
That may be a follow-up -- there's still several of these things I'm
looking at. I wanted to go ahead and start pushing out those which were
clearly improvements rather than queue them while I looked at all the
oddities I'm seeing in the dumps.
jeff

On Mon, Apr 8, 2013 at 3:27 PM, Jeff Law <law@redhat.com> wrote:
> On 04/08/2013 03:45 AM, Richard Biener wrote:>>>> @@ -8584,6 +8584,43 @@ simplify_cond_using_ranges (gimple stmt)>>> }>>> }>>>>>> + /* If we have a comparison of a SSA_NAME boolean against>>> + a constant (which obviously must be [0..1]). See if the>>> + SSA_NAME was set by a type conversion where the source>>> + of the conversion is another SSA_NAME with a range [0..1].>>> +>>> + If so, we can replace the SSA_NAME in the comparison with>>> + the RHS of the conversion. This will often make the type>>> + conversion dead code which DCE will clean up. */>>> + if (TREE_CODE (op0) == SSA_NAME>>> + && TREE_CODE (TREE_TYPE (op0)) == BOOLEAN_TYPE>>>>>> Use>>>> (TREE_CODE (TREE_TYPE (op0)) == BOOLEAN_TYPE>> || (INTEGRAL_TYPE_P (TREE_TYPE (op0))>> && TYPE_PRECISION (TREE_TYPE (op0)) == 1))>>>> to catch some more cases.>> Good catch. Done.>>>>>>> + && is_gimple_min_invariant (op1))>>>>>> In this case it's simpler to test TREE_CODE (op1) == INTEGER_CST.>> Agreed & fixed.>>>>>>> + {>>> + gimple def_stmt = SSA_NAME_DEF_STMT (op0);>>> + tree innerop;>>> +>>> + if (!is_gimple_assign (def_stmt)>>> + || !CONVERT_EXPR_CODE_P (gimple_assign_rhs_code (def_stmt)))>>> + return false;>>> +>>> + innerop = gimple_assign_rhs1 (def_stmt);>>> +>>> + if (!SSA_NAME_OCCURS_IN_ABNORMAL_PHI (innerop))>>>>>> As Steven said, the abnormal check is not necessary, but for completeness>> you should check TREE_CODE (innerop) == SSA_NAME. Valid (but>> unfolded) GIMPLE can have (_Bool) 1, too.>> Agreed & fixed.>>>>>> Note that we already have code with similar functionality (see if a>> conversion would alter the value of X) as part of optimizing>> (T1)(T2)X to (T1)X in simplify_conversion_using_ranges. Maybe>> a part of it can be split out and used to simplify conditions for>> a bigger range of types than just compares against boolean 0/1.>> That may be a follow-up -- there's still several of these things I'm looking> at. I wanted to go ahead and start pushing out those which were clearly> improvements rather than queue them while I looked at all the oddities I'm> seeing in the dumps.
Fine with me.
Richard.
> jeff>