PATCH: PR middle-end/47383: ivopts miscompiles Pmode != ptr_mode
Richard Guenther
richard.guenther@gmail.com
Wed Feb 9 23:30:00 GMT 2011
On Thu, Feb 10, 2011 at 12:05 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Wed, Feb 9, 2011 at 1:35 PM, Richard Guenther
> <richard.guenther@gmail.com> wrote:
>> On Wed, Feb 9, 2011 at 10:28 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>> On Wed, Feb 9, 2011 at 1:22 PM, Richard Guenther
>>> <richard.guenther@gmail.com> wrote:
>>>> On Wed, Feb 9, 2011 at 10:17 PM, Richard Guenther
>>>> <richard.guenther@gmail.com> wrote:
>>>>> On Wed, Feb 9, 2011 at 9:45 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>>>>> On Wed, Feb 9, 2011 at 12:17 PM, Richard Guenther
>>>>>> <richard.guenther@gmail.com> wrote:
>>>>>>> On Wed, Feb 9, 2011 at 6:17 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>>>>>>> On Wed, Feb 9, 2011 at 2:04 AM, Richard Guenther
>>>>>>>> <richard.guenther@gmail.com> wrote:
>>>>>>>>> On Wed, Feb 9, 2011 at 12:08 AM, H.J. Lu <hongjiu.lu@intel.com> wrote:
>>>>>>>>>> Hi,
>>>>>>>>>>
>>>>>>>>>> tree-ssa-loop-ivopts.c has
>>>>>>>>>>
>>>>>>>>>> ---
>>>>>>>>>> /* Returns variant of TYPE that can be used as base for different uses.
>>>>>>>>>> We return unsigned type with the same precision, which avoids problems
>>>>>>>>>> with overflows. */
>>>>>>>>>>
>>>>>>>>>> static tree
>>>>>>>>>> generic_type_for (tree type)
>>>>>>>>>> {
>>>>>>>>>> if (POINTER_TYPE_P (type))
>>>>>>>>>> return unsigned_type_for (type);
>>>>>>>>>>
>>>>>>>>>> if (TYPE_UNSIGNED (type))
>>>>>>>>>> return type;
>>>>>>>>>>
>>>>>>>>>> return unsigned_type_for (type);
>>>>>>>>>> }
>>>>>>>>>> ---
>>>>>>>>>>
>>>>>>>>>> It converts negative step to unsigned type with the same precision.
>>>>>>>>>> It doesn't work when Pmode != ptr_mode. This patch disables it.
>>>>>>>>>> OK for trunk in stage 1?
>>>>>>>>>
>>>>>>>>> This does not look correct. It rather sounds you are expanding
>>>>>>>>> TARGET_MEM_REFs the wrong way - the address computed by
>>>>>>>>> it is supposed to be calculated in ptr_mode and only the final
>>>>>>>>> result extended to Pmode.
>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> TARGET_MEM_REF is OK. The problem is ivopts passes
>>>>>>>> the negative offset to TARGET_MEM_REF as unsigned and
>>>>>>>> expects offset will wrap.
>>>>>>>
>>>>>>> But they will wrap, as arithmetic is carried out in a type with
>>>>>>> the same precision (that of ptr_mode).
>>>>>>>
>>>>>>>> It only works when Pmode ==
>>>>>>>> ptr_mode. I am checking in this patch into x32 branch to
>>>>>>>> avoid such cases.
>>>>>>>
>>>>>>> I think you are wrong.
>>>>>>>
>>>>>>
>>>>>> If you have a patch, I can give a try.
>>>>>
>>>>> I'd have to debug it and I don't have a target that shows the failure, but
>>>>> just looking around I assume that in
>>>>>
>>>>> rtx
>>>>> addr_for_mem_ref (struct mem_address *addr, addr_space_t as,
>>>>> bool really_expand)
>>>>> {
>>>>> enum machine_mode address_mode = targetm.addr_space.address_mode (as);
>>>>>
>>>>> address_mode will be Pmode - that would be already wrong. We need to
>>>>> use ptr_mode here and at the end of the function convert the
>>>>> generated address to Pmode appropriately. Can you verify that theory?
>>>>
>>>> From the default hook implementation that indeed seems to be the case.
>>>> Using ptr_mode might be undesirable as then the generated RTL will
>>>> probably never match complex addressing modes. Thus the component
>>>> and address types used for TARGET_MEM_REF are not suitable for
>>>> a Pmode != ptr_mode target and would have to use a pointer type
>>>> of Pmode mode (which we don't have, and frankly introducing such a
>>>> 2nd pointer mode on trees might have non-trivial effects). Eventually
>>>> just forcing the offset components of TARGET_MEM_REF to Pmode
>>>> size would also be a possibility (though you will run into the issue that
>>>> we loose any signedness of pointer offsets by casting them to sizetype).
>>>
>>> The offset components have to be signed while ptr_mode to Pmode
>>> may be zero extended.
>>
>> The sign of the offset components does not matter if they have the
>> same width as the resulting pointer. Or do you say we have
>> a %eax + %ebx addressing mode that zero-extends eax and sign-extends
>> ebx? Or that we can use a 32bit offset register that will be sign-extended?
>
> x32 uses %rax + %rbx * scale. RAX is zero-extended and
> RBX is sign-extended.
how can %rax or %rbx be extended further? Those are 64bit registers.
Richard.
More information about the Gcc-patches
mailing list