Fix recent thinko in operand_equal_p
Checks
Commit Message
Hi,
there is a thinko in the recent improvement by Jan:
2020-11-19 Jan Hubicka <jh@suse.cz>
* fold-const.c (operand_compare::operand_equal_p): Fix thinko in
COMPONENT_REF handling and guard types_same_for_odr by
virtual_method_call_p.
(operand_compare::hash_operand): Likewise.
where the code just looks at operand 2 of COMPONENT_REF, if it is present, to
compare addresses. That's wrong because operand 2 contains the number of
DECL_OFFSET_ALIGN-bit-sized words so, when DECL_OFFSET_ALIGN > 8, not all the
bytes are included and some of them are in DECL_FIELD_BIT_OFFSET instead, see
get_inner_reference for the model computation.
In other words, you would need to compare operand 2 and DECL_OFFSET_ALIGN and
DECL_FIELD_BIT_OFFSET in this situation, but I'm not sure this is worth the
hassle in practice so the attached fix just removes this alternate handling.
Tested on x86-64/Linux, OK for mainline, 12 and 11 branches?
2022-11-04 Eric Botcazou <ebotcazou@adacore.com>
* fold-const.cc (operand_compare::operand_equal_p) <COMPONENT_REF>:
Do not take into account operand 2.
(operand_compare::hash_operand) <COMPONENT_REF>: Likewise.
2022-11-04 Eric Botcazou <ebotcazou@adacore.com>
* gnat.dg/opt99.adb: New test.
* gnat.dg/opt99_pkg1.ads, gnat.dg/opt99_pkg1.adb: New helper.
* gnat.dg/opt99_pkg2.ads: Likewise.
Comments
> Am 04.11.2022 um 10:29 schrieb Eric Botcazou via Gcc-patches <gcc-patches@gcc.gnu.org>:
>
> Hi,
>
> there is a thinko in the recent improvement by Jan:
>
> 2020-11-19 Jan Hubicka <jh@suse.cz>
>
> * fold-const.c (operand_compare::operand_equal_p): Fix thinko in
> COMPONENT_REF handling and guard types_same_for_odr by
> virtual_method_call_p.
> (operand_compare::hash_operand): Likewise.
>
> where the code just looks at operand 2 of COMPONENT_REF, if it is present, to
> compare addresses. That's wrong because operand 2 contains the number of
> DECL_OFFSET_ALIGN-bit-sized words so, when DECL_OFFSET_ALIGN > 8, not all the
> bytes are included and some of them are in DECL_FIELD_BIT_OFFSET instead, see
> get_inner_reference for the model computation.
>
> In other words, you would need to compare operand 2 and DECL_OFFSET_ALIGN and
> DECL_FIELD_BIT_OFFSET in this situation, but I'm not sure this is worth the
> hassle in practice so the attached fix just removes this alternate handling.
> Tested on x86-64/Linux, OK for mainline, 12 and 11 branches?
Ok.
Thanks,
Richard
>
> 2022-11-04 Eric Botcazou <ebotcazou@adacore.com>
>
> * fold-const.cc (operand_compare::operand_equal_p) <COMPONENT_REF>:
> Do not take into account operand 2.
> (operand_compare::hash_operand) <COMPONENT_REF>: Likewise.
>
>
> 2022-11-04 Eric Botcazou <ebotcazou@adacore.com>
>
> * gnat.dg/opt99.adb: New test.
> * gnat.dg/opt99_pkg1.ads, gnat.dg/opt99_pkg1.adb: New helper.
> * gnat.dg/opt99_pkg2.ads: Likewise.
>
> --
> Eric Botcazou
> <p.diff>
> <opt99.adb>
> <opt99_pkg1.adb>
> <opt99_pkg2.ads>
> <opt99_pkg1.ads>
@@ -3351,9 +3351,6 @@ operand_compare::operand_equal_p (const_tree arg0, const_tree arg1,
if (compare_address
&& (flags & OEP_ADDRESS_OF_SAME_FIELD) == 0)
{
- if (TREE_OPERAND (arg0, 2)
- || TREE_OPERAND (arg1, 2))
- return OP_SAME_WITH_NULL (2);
tree field0 = TREE_OPERAND (arg0, 1);
tree field1 = TREE_OPERAND (arg1, 1);
@@ -3864,17 +3861,10 @@ operand_compare::hash_operand (const_tree t, inchash::hash &hstate,
if (sflags & OEP_ADDRESS_OF)
{
hash_operand (TREE_OPERAND (t, 0), hstate, flags);
- if (TREE_OPERAND (t, 2))
- hash_operand (TREE_OPERAND (t, 2), hstate,
- flags & ~OEP_ADDRESS_OF);
- else
- {
- tree field = TREE_OPERAND (t, 1);
- hash_operand (DECL_FIELD_OFFSET (field),
- hstate, flags & ~OEP_ADDRESS_OF);
- hash_operand (DECL_FIELD_BIT_OFFSET (field),
- hstate, flags & ~OEP_ADDRESS_OF);
- }
+ hash_operand (DECL_FIELD_OFFSET (TREE_OPERAND (t, 1)),
+ hstate, flags & ~OEP_ADDRESS_OF);
+ hash_operand (DECL_FIELD_BIT_OFFSET (TREE_OPERAND (t, 1)),
+ hstate, flags & ~OEP_ADDRESS_OF);
return;
}
break;