PATCH: PR middle-end/47383: ivopts miscompiles Pmode != ptr_mode
H.J. Lu
hjl.tools@gmail.com
Sat Feb 12 17:30:00 GMT 2011
On Wed, Feb 9, 2011 at 1: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?
>
This patch works for me. However, if I add
diff --git a/gcc/tree-ssa-address.c b/gcc/tree-ssa-address.c
index 7b7e169..d9d906e 100644
--- a/gcc/tree-ssa-address.c
+++ b/gcc/tree-ssa-address.c
@@ -242,6 +242,9 @@ addr_for_mem_ref (struct mem_address *addr, addr_space_t as,
if (off)
*templ->off_p = off;
+ if (targetm.addr_space.address_mode (as) != ptr_mode)
+ templ->ref = convert_to_mode (targetm.addr_space.address_mode (as),
+ templ->ref, 1);
return templ->ref;
}
to convert MEM_REF from ptr_mode to Pmode, I get
/export/build/gnu/gcc-x32/build-x86_64-linux/gcc/xgcc
-B/export/build/gnu/gcc-x32/build-x86_64-linux/gcc/ -S -o x.s -mx32
-O2 -g -fPIC x.i
x.i: In function \u2018foo\u2019:
x.i:21:1: error: insn outside basic block
(insn 88 0 113 (parallel [
(set (reg:SI 143)
(ashift:SI (reg:SI 60)
(const_int 3 [0x3])))
(clobber (reg:CC 17 flags))
]) -1
(nil))
x.i:21:1: internal compiler error: in rtl_verify_flow_info, at cfgrtl.c:2218
Please submit a full bug report,
with preprocessed source if appropriate.
See <http://gcc.gnu.org/bugs.html> for instructions.
make: *** [x.s] Error 1
Do I really need to convert MEM_REF from ptr_mode to Pmode?
Thanks.
--
H.J.
---
diff --git a/gcc/tree-ssa-address.c b/gcc/tree-ssa-address.c
index a9ca835..7b7e169 100644
--- a/gcc/tree-ssa-address.c
+++ b/gcc/tree-ssa-address.c
@@ -188,12 +188,12 @@ 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);
rtx address, sym, bse, idx, st, off;
struct mem_addr_template *templ;
if (addr->step && !integer_onep (addr->step))
- st = immed_double_int_const (tree_to_double_int (addr->step), address_mode)
;
+ st = immed_double_int_const (tree_to_double_int (addr->step),
+ ptr_mode);
else
st = NULL_RTX;
@@ -201,7 +201,7 @@ addr_for_mem_ref (struct mem_address *addr, addr_space_t as,
off = immed_double_int_const
(double_int_sext (tree_to_double_int (addr->offset),
TYPE_PRECISION (TREE_TYPE (addr->offset))),
- address_mode);
+ ptr_mode);
else
off = NULL_RTX;
@@ -220,16 +220,16 @@ addr_for_mem_ref (struct mem_address *addr, addr_space_t a
s,
if (!templ->ref)
{
sym = (addr->symbol ?
- gen_rtx_SYMBOL_REF (address_mode, ggc_strdup ("test_symbol"))
+ gen_rtx_SYMBOL_REF (ptr_mode, ggc_strdup ("test_symbol"))
: NULL_RTX);
bse = (addr->base ?
- gen_raw_REG (address_mode, LAST_VIRTUAL_REGISTER + 1)
+ gen_raw_REG (ptr_mode, LAST_VIRTUAL_REGISTER + 1)
: NULL_RTX);
idx = (addr->index ?
- gen_raw_REG (address_mode, LAST_VIRTUAL_REGISTER + 2)
+ gen_raw_REG (ptr_mode, LAST_VIRTUAL_REGISTER + 2)
: NULL_RTX);
- gen_addr_rtx (address_mode, sym, bse, idx,
+ gen_addr_rtx (ptr_mode, sym, bse, idx,
st? const0_rtx : NULL_RTX,
off? const0_rtx : NULL_RTX,
&templ->ref,
@@ -247,16 +247,16 @@ addr_for_mem_ref (struct mem_address *addr, addr_space_t a
s,
/* Otherwise really expand the expressions. */
sym = (addr->symbol
- ? expand_expr (addr->symbol, NULL_RTX, address_mode, EXPAND_NORMAL)
+ ? expand_expr (addr->symbol, NULL_RTX, ptr_mode, EXPAND_NORMAL)
: NULL_RTX);
bse = (addr->base
- ? expand_expr (addr->base, NULL_RTX, address_mode, EXPAND_NORMAL)
+ ? expand_expr (addr->base, NULL_RTX, ptr_mode, EXPAND_NORMAL)
: NULL_RTX);
idx = (addr->index
- ? expand_expr (addr->index, NULL_RTX, address_mode, EXPAND_NORMAL)
+ ? expand_expr (addr->index, NULL_RTX, ptr_mode, EXPAND_NORMAL)
: NULL_RTX);
- gen_addr_rtx (address_mode, sym, bse, idx, st, off, &address, NULL, NULL);
+ gen_addr_rtx (ptr_mode, sym, bse, idx, st, off, &address, NULL, NULL);
return address;
}
More information about the Gcc-patches
mailing list