On 6/26/19 8:40 PM, Martin Sebor wrote:
> On 6/26/19 4:31 PM, Jeff Law wrote:
>> On 6/25/19 5:03 PM, Martin Sebor wrote:
>>
>>>
>>> The caller ensures that handle_char_store is only called for stores
>>> to arrays (MEM_REF) or single elements as wide as char.
>> Where? I don't see it, even after fixing the formatting in
>> strlen_check_and_optimize_stmt :-)
>>
>>> gimple *stmt = gsi_stmt (*gsi);
>>>
>>> if (is_gimple_call (stmt))
>>
>> I think we can agree that we don't have a call, so this is false.
>>
>>> else if (is_gimple_assign (stmt) && !gimple_clobber_p (stmt))
>>> {
>>> tree lhs = gimple_assign_lhs (stmt);
>> This should be true IIUC, so we'll go into its THEN block.
>>
>>
>>> if (TREE_CODE (lhs) == SSA_NAME && POINTER_TYPE_P (TREE_TYPE
>>> (lhs)))
>> Should be false.
>>
>>> else if (TREE_CODE (lhs) == SSA_NAME && INTEGRAL_TYPE_P
>>> (TREE_TYPE (lhs)))
>>
>> Should also be false.
>>
>>> else if (TREE_CODE (lhs) != SSA_NAME && !TREE_SIDE_EFFECTS (lhs))
>> Should be true since LHS is a MEM_REF.
>>
>>
>>> {
>>> tree type = TREE_TYPE (lhs);
>>> if (TREE_CODE (type) == ARRAY_TYPE)
>>> type = TREE_TYPE (type);
>>> if (TREE_CODE (type) == INTEGER_TYPE
>>> && TYPE_MODE (type) == TYPE_MODE (char_type_node)
>>> && TYPE_PRECISION (type) == TYPE_PRECISION
>>> (char_type_node))
>>> {
>>> if (! handle_char_store (gsi))
>>> return false;
>>> }
>>> }
>> If TREE_TYPE (type) is an ARRAY_TYPE, we strip the ARRAY_TYPE. We then
>> verify that TYPE is a single character type. _But_ we stripped away the
>> ARRAY_TYPE. So ISTM that we allow either an array of chars or a single
>> char on the LHS.
>>
>> So how does this avoid multiple character stores?!? We could have had
>> an ARRAY_REF on the LHS and we never check the number of elements in the
>> array. There's no check on the RHS either. SO I don't see how we
>> guarantee that we're dealing with a single character store.
>>
>> What am I missing here?
>
> Can you show me a test case where it breaks? If not, I don't know
> what you want me to do. I'll just move on to something else.
THe thing to do is research what gimple accepts and what other passes
may do. Given this is a code generation bug, "just moving on" isn't
really a good option unless we're going to rip out that little bit of code.
As I was thinking about this last night, the pass we'd want to look at
would be store merging. Let's do that on the call today.
jeff
