Re: PATCH: PR rtl-optimization/64037: Miscompilation with -Os and enum class : char parameter
On Sun, Dec 14, 2014 at 3:08 PM, H.J. Lu hjl.to...@gmail.com wrote:
The enclosed testcase fails on x86 when compiled with -Os since we pass
a byte parameter with a byte load in caller and read it as an int in
callee. The reason it only shows up with -Os is x86 backend encodes
a byte load with an int load if -O isn't used. When a byte load is
used, the upper 24 bits of the register have random value for none
WORD_REGISTER_OPERATIONS targets.
It happens because setup_incoming_promotions in combine.c has
/* The mode and signedness of the argument before any promotions
happen
(equal to the mode of the pseudo holding it at that stage). */
mode1 = TYPE_MODE (TREE_TYPE (arg));
uns1 = TYPE_UNSIGNED (TREE_TYPE (arg));
/* The mode and signedness of the argument after any source
language and
TARGET_PROMOTE_PROTOTYPES-driven promotions. */
mode2 = TYPE_MODE (DECL_ARG_TYPE (arg));
uns3 = TYPE_UNSIGNED (DECL_ARG_TYPE (arg));
/* The mode and signedness of the argument as it is actually
passed,
after any TARGET_PROMOTE_FUNCTION_ARGS-driven ABI promotions.
*/
mode3 = promote_function_mode (DECL_ARG_TYPE (arg), mode2, uns3,
TREE_TYPE (cfun-decl), 0);
while they are actually passed in register by assign_parm_setup_reg in
function.c:
/* Store the parm in a pseudoregister during the function, but we may
need to do it in a wider mode. Using 2 here makes the result
consistent with promote_decl_mode and thus expand_expr_real_1. */
promoted_nominal_mode
= promote_function_mode (data-nominal_type, data-nominal_mode,
unsignedp,
TREE_TYPE (current_function_decl), 2);
where nominal_type and nominal_mode are set up with TREE_TYPE (parm)
and TYPE_MODE (nominal_type). TREE_TYPE here is
I think the bug is here, not in combine.c. Can you try going back in
history
for both snippets and see if they matched at some point?
The bug was introduced by
https://gcc.gnu.org/ml/gcc-cvs/2007-09/msg00613.html
commit 5d93234932c3d8617ce92b77b7013ef6bede9508
Author: shinwell shinwell@138bc75d-0d04-0410-961f-82ee72b054a4
Date: Thu Sep 20 11:01:18 2007 +
gcc/
* combine.c: Include cgraph.h.
(setup_incoming_promotions): Rework to allow more aggressive
elimination of sign extensions when all call sites of the
current function are known to lie within the current unit.
git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@128618
138bc75d-0d04-0410-961f-82ee72b054a4
Before this commit, combine.c has
enum machine_mode mode = TYPE_MODE (TREE_TYPE (arg));
int uns = TYPE_UNSIGNED (TREE_TYPE (arg));
mode = promote_mode (TREE_TYPE (arg), mode, uns, 1);
if (mode == GET_MODE (reg) mode != DECL_MODE (arg))
{
rtx x;
x = gen_rtx_CLOBBER (DECL_MODE (arg), const0_rtx);
x = gen_rtx_fmt_e ((uns ? ZERO_EXTEND : SIGN_EXTEND), mode,
x);
record_value_for_reg (reg, first, x);
}
It matches function.c:
/* This is not really promoting for a call. However we need to be
consistent with assign_parm_find_data_types and expand_expr_real_1.
*/
promoted_nominal_mode
= promote_mode (data-nominal_type, data-nominal_mode, unsignedp,
1);
r128618 changed
mode = promote_mode (TREE_TYPE (arg), mode, uns, 1);
to
mode3 = promote_mode (DECL_ARG_TYPE (arg), mode2, uns3, 1);
It breaks none WORD_REGISTER_OPERATIONS targets.
Hmm, I think that DECL_ARG_TYPE makes a difference only
for non-WORD_REGISTER_OPERATIONS targets.
But yeah, isolated the above change looks wrong.
Your patch is ok for trunk if nobody objects within 24h and for branches
after a week.
Thanks,
Richard.
This patch caused PR64213.
Here is the updated patch. The difference is
mode3 = promote_function_mode (TREE_TYPE (arg), mode1, uns3,
TREE_TYPE (cfun-decl), 0);
vs
mode3 = promote_function_mode (TREE_TYPE (arg), mode1, uns1,
TREE_TYPE (cfun-decl), 0);
I made a mistake in my previous patch where I shouldn't have changed
uns3 to uns1. We do want to update mode3 and uns3, not mode3 and
uns1. It generates the same code on PR64213 testcase with a cross
alpha-linux GCC.
Uros, can you test it on Linux/alpha? OK for master, 4.9 and 4.8
branches if it works on Linux/alpha?
Yes, this patch works OK [1] on linux/alpha mainline.
[1] https://gcc.gnu.org/ml/gcc-testresults/2014-12/msg01867.html
Uros.
Re: PATCH: PR rtl-optimization/64037: Miscompilation with -Os and enum class : char parameter
On Mon, Dec 15, 2014 at 9:29 AM, Uros Bizjak ubiz...@gmail.com wrote:
The enclosed testcase fails on x86 when compiled with -Os since we pass
a byte parameter with a byte load in caller and read it as an int in
callee. The reason it only shows up with -Os is x86 backend encodes
a byte load with an int load if -O isn't used. When a byte load is
used, the upper 24 bits of the register have random value for none
WORD_REGISTER_OPERATIONS targets.
It happens because setup_incoming_promotions in combine.c has
/* The mode and signedness of the argument before any promotions
happen
(equal to the mode of the pseudo holding it at that stage).
*/
mode1 = TYPE_MODE (TREE_TYPE (arg));
uns1 = TYPE_UNSIGNED (TREE_TYPE (arg));
/* The mode and signedness of the argument after any source
language and
TARGET_PROMOTE_PROTOTYPES-driven promotions. */
mode2 = TYPE_MODE (DECL_ARG_TYPE (arg));
uns3 = TYPE_UNSIGNED (DECL_ARG_TYPE (arg));
/* The mode and signedness of the argument as it is actually
passed,
after any TARGET_PROMOTE_FUNCTION_ARGS-driven ABI promotions.
*/
mode3 = promote_function_mode (DECL_ARG_TYPE (arg), mode2, uns3,
TREE_TYPE (cfun-decl), 0);
while they are actually passed in register by assign_parm_setup_reg in
function.c:
/* Store the parm in a pseudoregister during the function, but we may
need to do it in a wider mode. Using 2 here makes the result
consistent with promote_decl_mode and thus expand_expr_real_1. */
promoted_nominal_mode
= promote_function_mode (data-nominal_type, data-nominal_mode,
unsignedp,
TREE_TYPE (current_function_decl), 2);
where nominal_type and nominal_mode are set up with TREE_TYPE (parm)
and TYPE_MODE (nominal_type). TREE_TYPE here is
I think the bug is here, not in combine.c. Can you try going back in
history
for both snippets and see if they matched at some point?
The bug was introduced by
https://gcc.gnu.org/ml/gcc-cvs/2007-09/msg00613.html
commit 5d93234932c3d8617ce92b77b7013ef6bede9508
Author: shinwell shinwell@138bc75d-0d04-0410-961f-82ee72b054a4
Date: Thu Sep 20 11:01:18 2007 +
gcc/
* combine.c: Include cgraph.h.
(setup_incoming_promotions): Rework to allow more aggressive
elimination of sign extensions when all call sites of the
current function are known to lie within the current unit.
git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@128618
138bc75d-0d04-0410-961f-82ee72b054a4
Before this commit, combine.c has
enum machine_mode mode = TYPE_MODE (TREE_TYPE (arg));
int uns = TYPE_UNSIGNED (TREE_TYPE (arg));
mode = promote_mode (TREE_TYPE (arg), mode, uns, 1);
if (mode == GET_MODE (reg) mode != DECL_MODE (arg))
{
rtx x;
x = gen_rtx_CLOBBER (DECL_MODE (arg), const0_rtx);
x = gen_rtx_fmt_e ((uns ? ZERO_EXTEND : SIGN_EXTEND),
mode, x);
record_value_for_reg (reg, first, x);
}
It matches function.c:
/* This is not really promoting for a call. However we need to be
consistent with assign_parm_find_data_types and expand_expr_real_1.
*/
promoted_nominal_mode
= promote_mode (data-nominal_type, data-nominal_mode, unsignedp,
1);
r128618 changed
mode = promote_mode (TREE_TYPE (arg), mode, uns, 1);
to
mode3 = promote_mode (DECL_ARG_TYPE (arg), mode2, uns3, 1);
It breaks none WORD_REGISTER_OPERATIONS targets.
Hmm, I think that DECL_ARG_TYPE makes a difference only
for non-WORD_REGISTER_OPERATIONS targets.
But yeah, isolated the above change looks wrong.
Your patch is ok for trunk if nobody objects within 24h and for branches
after a week.
Thanks,
Richard.
This patch caused PR64213.
Here is the updated patch. The difference is
mode3 = promote_function_mode (TREE_TYPE (arg), mode1, uns3,
TREE_TYPE (cfun-decl), 0);
vs
mode3 = promote_function_mode (TREE_TYPE (arg), mode1, uns1,
TREE_TYPE (cfun-decl), 0);
I made a mistake in my previous patch where I shouldn't have changed
uns3 to uns1. We do want to update mode3 and uns3, not mode3 and
uns1. It generates the same code on PR64213 testcase with a cross
alpha-linux GCC.
Uros, can you test it on Linux/alpha? OK for master, 4.9 and 4.8
branches if it works on Linux/alpha?
Yes, this patch works OK [1] on linux/alpha mainline.
... and 4.9 branch [2].
[1] https://gcc.gnu.org/ml/gcc-testresults/2014-12/msg01867.html
[2] https://gcc.gnu.org/ml/gcc-testresults/2014-12/msg01919.html
Uros.
Re: PATCH: PR rtl-optimization/64037: Miscompilation with -Os and enum class : char parameter
On Mon, Dec 08, 2014 at 11:16:58PM +0100, Uros Bizjak wrote:
Hello!
On Tue, Nov 25, 2014 at 5:05 PM, H.J. Lu hjl.to...@gmail.com wrote:
On Tue, Nov 25, 2014 at 7:01 AM, Richard Biener
richard.guent...@gmail.com wrote:
On Tue, Nov 25, 2014 at 1:57 PM, H.J. Lu hongjiu...@intel.com wrote:
Hi,
The enclosed testcase fails on x86 when compiled with -Os since we pass
a byte parameter with a byte load in caller and read it as an int in
callee. The reason it only shows up with -Os is x86 backend encodes
a byte load with an int load if -O isn't used. When a byte load is
used, the upper 24 bits of the register have random value for none
WORD_REGISTER_OPERATIONS targets.
It happens because setup_incoming_promotions in combine.c has
/* The mode and signedness of the argument before any promotions
happen
(equal to the mode of the pseudo holding it at that stage). */
mode1 = TYPE_MODE (TREE_TYPE (arg));
uns1 = TYPE_UNSIGNED (TREE_TYPE (arg));
/* The mode and signedness of the argument after any source
language and
TARGET_PROMOTE_PROTOTYPES-driven promotions. */
mode2 = TYPE_MODE (DECL_ARG_TYPE (arg));
uns3 = TYPE_UNSIGNED (DECL_ARG_TYPE (arg));
/* The mode and signedness of the argument as it is actually
passed,
after any TARGET_PROMOTE_FUNCTION_ARGS-driven ABI promotions.
*/
mode3 = promote_function_mode (DECL_ARG_TYPE (arg), mode2, uns3,
TREE_TYPE (cfun-decl), 0);
while they are actually passed in register by assign_parm_setup_reg in
function.c:
/* Store the parm in a pseudoregister during the function, but we may
need to do it in a wider mode. Using 2 here makes the result
consistent with promote_decl_mode and thus expand_expr_real_1. */
promoted_nominal_mode
= promote_function_mode (data-nominal_type, data-nominal_mode,
unsignedp,
TREE_TYPE (current_function_decl), 2);
where nominal_type and nominal_mode are set up with TREE_TYPE (parm)
and TYPE_MODE (nominal_type). TREE_TYPE here is
I think the bug is here, not in combine.c. Can you try going back in
history
for both snippets and see if they matched at some point?
The bug was introduced by
https://gcc.gnu.org/ml/gcc-cvs/2007-09/msg00613.html
commit 5d93234932c3d8617ce92b77b7013ef6bede9508
Author: shinwell shinwell@138bc75d-0d04-0410-961f-82ee72b054a4
Date: Thu Sep 20 11:01:18 2007 +
gcc/
* combine.c: Include cgraph.h.
(setup_incoming_promotions): Rework to allow more aggressive
elimination of sign extensions when all call sites of the
current function are known to lie within the current unit.
git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@128618
138bc75d-0d04-0410-961f-82ee72b054a4
Before this commit, combine.c has
enum machine_mode mode = TYPE_MODE (TREE_TYPE (arg));
int uns = TYPE_UNSIGNED (TREE_TYPE (arg));
mode = promote_mode (TREE_TYPE (arg), mode, uns, 1);
if (mode == GET_MODE (reg) mode != DECL_MODE (arg))
{
rtx x;
x = gen_rtx_CLOBBER (DECL_MODE (arg), const0_rtx);
x = gen_rtx_fmt_e ((uns ? ZERO_EXTEND : SIGN_EXTEND), mode,
x);
record_value_for_reg (reg, first, x);
}
It matches function.c:
/* This is not really promoting for a call. However we need to be
consistent with assign_parm_find_data_types and expand_expr_real_1.
*/
promoted_nominal_mode
= promote_mode (data-nominal_type, data-nominal_mode, unsignedp, 1);
r128618 changed
mode = promote_mode (TREE_TYPE (arg), mode, uns, 1);
to
mode3 = promote_mode (DECL_ARG_TYPE (arg), mode2, uns3, 1);
It breaks none WORD_REGISTER_OPERATIONS targets.
Hmm, I think that DECL_ARG_TYPE makes a difference only
for non-WORD_REGISTER_OPERATIONS targets.
But yeah, isolated the above change looks wrong.
Your patch is ok for trunk if nobody objects within 24h and for branches
after a week.
Thanks,
Richard.
This patch caused PR64213.
Here is the updated patch. The difference is
mode3 = promote_function_mode (TREE_TYPE (arg), mode1, uns3,
TREE_TYPE (cfun-decl), 0);
vs
mode3 = promote_function_mode (TREE_TYPE (arg), mode1, uns1,
TREE_TYPE (cfun-decl), 0);
I made a mistake in my previous patch where I shouldn't have changed
uns3 to uns1. We do want to update mode3 and uns3, not mode3 and
uns1. It generates the same code on PR64213 testcase with a cross
alpha-linux GCC.
Uros, can you test it on Linux/alpha? OK for master, 4.9 and 4.8
branches if it works on Linux/alpha?
Thanks.
H.J.
---
From 7b274d517dcaae96f111652283d947c035ab7a22 Mon Sep 17 00:00:00 2001
Re: PATCH: PR rtl-optimization/64037: Miscompilation with -Os and enum class : char parameter
On December 14, 2014 3:08:28 PM CET, H.J. Lu hjl.to...@gmail.com wrote:
On Mon, Dec 08, 2014 at 11:16:58PM +0100, Uros Bizjak wrote:
Hello!
On Tue, Nov 25, 2014 at 5:05 PM, H.J. Lu hjl.to...@gmail.com
wrote:
On Tue, Nov 25, 2014 at 7:01 AM, Richard Biener
richard.guent...@gmail.com wrote:
On Tue, Nov 25, 2014 at 1:57 PM, H.J. Lu hongjiu...@intel.com
wrote:
Hi,
The enclosed testcase fails on x86 when compiled with -Os since
we pass
a byte parameter with a byte load in caller and read it as an
int in
callee. The reason it only shows up with -Os is x86 backend
encodes
a byte load with an int load if -O isn't used. When a byte load
is
used, the upper 24 bits of the register have random value for
none
WORD_REGISTER_OPERATIONS targets.
It happens because setup_incoming_promotions in combine.c has
/* The mode and signedness of the argument before any
promotions happen
(equal to the mode of the pseudo holding it at that
stage). */
mode1 = TYPE_MODE (TREE_TYPE (arg));
uns1 = TYPE_UNSIGNED (TREE_TYPE (arg));
/* The mode and signedness of the argument after any
source language and
TARGET_PROMOTE_PROTOTYPES-driven promotions. */
mode2 = TYPE_MODE (DECL_ARG_TYPE (arg));
uns3 = TYPE_UNSIGNED (DECL_ARG_TYPE (arg));
/* The mode and signedness of the argument as it is
actually passed,
after any TARGET_PROMOTE_FUNCTION_ARGS-driven ABI
promotions. */
mode3 = promote_function_mode (DECL_ARG_TYPE (arg), mode2,
uns3,
TREE_TYPE (cfun-decl), 0);
while they are actually passed in register by
assign_parm_setup_reg in
function.c:
/* Store the parm in a pseudoregister during the function, but
we may
need to do it in a wider mode. Using 2 here makes the
result
consistent with promote_decl_mode and thus
expand_expr_real_1. */
promoted_nominal_mode
= promote_function_mode (data-nominal_type,
data-nominal_mode, unsignedp,
TREE_TYPE (current_function_decl),
2);
where nominal_type and nominal_mode are set up with TREE_TYPE
(parm)
and TYPE_MODE (nominal_type). TREE_TYPE here is
I think the bug is here, not in combine.c. Can you try going
back in history
for both snippets and see if they matched at some point?
The bug was introduced by
https://gcc.gnu.org/ml/gcc-cvs/2007-09/msg00613.html
commit 5d93234932c3d8617ce92b77b7013ef6bede9508
Author: shinwell shinwell@138bc75d-0d04-0410-961f-82ee72b054a4
Date: Thu Sep 20 11:01:18 2007 +
gcc/
* combine.c: Include cgraph.h.
(setup_incoming_promotions): Rework to allow more aggressive
elimination of sign extensions when all call sites of the
current function are known to lie within the current unit.
git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@128618
138bc75d-0d04-0410-961f-82ee72b054a4
Before this commit, combine.c has
enum machine_mode mode = TYPE_MODE (TREE_TYPE (arg));
int uns = TYPE_UNSIGNED (TREE_TYPE (arg));
mode = promote_mode (TREE_TYPE (arg), mode, uns, 1);
if (mode == GET_MODE (reg) mode != DECL_MODE (arg))
{
rtx x;
x = gen_rtx_CLOBBER (DECL_MODE (arg), const0_rtx);
x = gen_rtx_fmt_e ((uns ? ZERO_EXTEND :
SIGN_EXTEND), mode, x);
record_value_for_reg (reg, first, x);
}
It matches function.c:
/* This is not really promoting for a call. However we need to
be
consistent with assign_parm_find_data_types and
expand_expr_real_1. */
promoted_nominal_mode
= promote_mode (data-nominal_type, data-nominal_mode,
unsignedp, 1);
r128618 changed
mode = promote_mode (TREE_TYPE (arg), mode, uns, 1);
to
mode3 = promote_mode (DECL_ARG_TYPE (arg), mode2, uns3, 1);
It breaks none WORD_REGISTER_OPERATIONS targets.
Hmm, I think that DECL_ARG_TYPE makes a difference only
for non-WORD_REGISTER_OPERATIONS targets.
But yeah, isolated the above change looks wrong.
Your patch is ok for trunk if nobody objects within 24h and for
branches
after a week.
Thanks,
Richard.
This patch caused PR64213.
Here is the updated patch. The difference is
mode3 = promote_function_mode (TREE_TYPE (arg), mode1, uns3,
TREE_TYPE (cfun-decl), 0);
vs
mode3 = promote_function_mode (TREE_TYPE (arg), mode1, uns1,
TREE_TYPE (cfun-decl), 0);
I made a mistake in my previous patch where I shouldn't have changed
uns3 to uns1. We do want to update mode3 and uns3, not mode3 and
uns1. It generates the same code on PR64213 testcase with a cross
alpha-linux GCC.
Uros, can you test it on Linux/alpha? OK for master, 4.9 and 4.8
branches if it works on Linux/alpha?
OK for trunk and 4.9, please wait until after
Re: PATCH: PR rtl-optimization/64037: Miscompilation with -Os and enum class : char parameter
Hello!
On Tue, Nov 25, 2014 at 5:05 PM, H.J. Lu hjl.to...@gmail.com wrote:
On Tue, Nov 25, 2014 at 7:01 AM, Richard Biener
richard.guent...@gmail.com wrote:
On Tue, Nov 25, 2014 at 1:57 PM, H.J. Lu hongjiu...@intel.com wrote:
Hi,
The enclosed testcase fails on x86 when compiled with -Os since we pass
a byte parameter with a byte load in caller and read it as an int in
callee. The reason it only shows up with -Os is x86 backend encodes
a byte load with an int load if -O isn't used. When a byte load is
used, the upper 24 bits of the register have random value for none
WORD_REGISTER_OPERATIONS targets.
It happens because setup_incoming_promotions in combine.c has
/* The mode and signedness of the argument before any promotions
happen
(equal to the mode of the pseudo holding it at that stage). */
mode1 = TYPE_MODE (TREE_TYPE (arg));
uns1 = TYPE_UNSIGNED (TREE_TYPE (arg));
/* The mode and signedness of the argument after any source language
and
TARGET_PROMOTE_PROTOTYPES-driven promotions. */
mode2 = TYPE_MODE (DECL_ARG_TYPE (arg));
uns3 = TYPE_UNSIGNED (DECL_ARG_TYPE (arg));
/* The mode and signedness of the argument as it is actually passed,
after any TARGET_PROMOTE_FUNCTION_ARGS-driven ABI promotions. */
mode3 = promote_function_mode (DECL_ARG_TYPE (arg), mode2, uns3,
TREE_TYPE (cfun-decl), 0);
while they are actually passed in register by assign_parm_setup_reg in
function.c:
/* Store the parm in a pseudoregister during the function, but we may
need to do it in a wider mode. Using 2 here makes the result
consistent with promote_decl_mode and thus expand_expr_real_1. */
promoted_nominal_mode
= promote_function_mode (data-nominal_type, data-nominal_mode,
unsignedp,
TREE_TYPE (current_function_decl), 2);
where nominal_type and nominal_mode are set up with TREE_TYPE (parm)
and TYPE_MODE (nominal_type). TREE_TYPE here is
I think the bug is here, not in combine.c. Can you try going back in
history
for both snippets and see if they matched at some point?
The bug was introduced by
https://gcc.gnu.org/ml/gcc-cvs/2007-09/msg00613.html
commit 5d93234932c3d8617ce92b77b7013ef6bede9508
Author: shinwell shinwell@138bc75d-0d04-0410-961f-82ee72b054a4
Date: Thu Sep 20 11:01:18 2007 +
gcc/
* combine.c: Include cgraph.h.
(setup_incoming_promotions): Rework to allow more aggressive
elimination of sign extensions when all call sites of the
current function are known to lie within the current unit.
git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@128618
138bc75d-0d04-0410-961f-82ee72b054a4
Before this commit, combine.c has
enum machine_mode mode = TYPE_MODE (TREE_TYPE (arg));
int uns = TYPE_UNSIGNED (TREE_TYPE (arg));
mode = promote_mode (TREE_TYPE (arg), mode, uns, 1);
if (mode == GET_MODE (reg) mode != DECL_MODE (arg))
{
rtx x;
x = gen_rtx_CLOBBER (DECL_MODE (arg), const0_rtx);
x = gen_rtx_fmt_e ((uns ? ZERO_EXTEND : SIGN_EXTEND), mode, x);
record_value_for_reg (reg, first, x);
}
It matches function.c:
/* This is not really promoting for a call. However we need to be
consistent with assign_parm_find_data_types and expand_expr_real_1. */
promoted_nominal_mode
= promote_mode (data-nominal_type, data-nominal_mode, unsignedp, 1);
r128618 changed
mode = promote_mode (TREE_TYPE (arg), mode, uns, 1);
to
mode3 = promote_mode (DECL_ARG_TYPE (arg), mode2, uns3, 1);
It breaks none WORD_REGISTER_OPERATIONS targets.
Hmm, I think that DECL_ARG_TYPE makes a difference only
for non-WORD_REGISTER_OPERATIONS targets.
But yeah, isolated the above change looks wrong.
Your patch is ok for trunk if nobody objects within 24h and for branches
after a week.
Thanks,
Richard.
This patch caused PR64213.
Uros.
Re: PATCH: PR rtl-optimization/64037: Miscompilation with -Os and enum class : char parameter
On Tue, Nov 25, 2014 at 5:05 PM, H.J. Lu hjl.to...@gmail.com wrote:
On Tue, Nov 25, 2014 at 7:01 AM, Richard Biener
richard.guent...@gmail.com wrote:
On Tue, Nov 25, 2014 at 1:57 PM, H.J. Lu hongjiu...@intel.com wrote:
Hi,
The enclosed testcase fails on x86 when compiled with -Os since we pass
a byte parameter with a byte load in caller and read it as an int in
callee. The reason it only shows up with -Os is x86 backend encodes
a byte load with an int load if -O isn't used. When a byte load is
used, the upper 24 bits of the register have random value for none
WORD_REGISTER_OPERATIONS targets.
It happens because setup_incoming_promotions in combine.c has
/* The mode and signedness of the argument before any promotions
happen
(equal to the mode of the pseudo holding it at that stage). */
mode1 = TYPE_MODE (TREE_TYPE (arg));
uns1 = TYPE_UNSIGNED (TREE_TYPE (arg));
/* The mode and signedness of the argument after any source language
and
TARGET_PROMOTE_PROTOTYPES-driven promotions. */
mode2 = TYPE_MODE (DECL_ARG_TYPE (arg));
uns3 = TYPE_UNSIGNED (DECL_ARG_TYPE (arg));
/* The mode and signedness of the argument as it is actually passed,
after any TARGET_PROMOTE_FUNCTION_ARGS-driven ABI promotions. */
mode3 = promote_function_mode (DECL_ARG_TYPE (arg), mode2, uns3,
TREE_TYPE (cfun-decl), 0);
while they are actually passed in register by assign_parm_setup_reg in
function.c:
/* Store the parm in a pseudoregister during the function, but we may
need to do it in a wider mode. Using 2 here makes the result
consistent with promote_decl_mode and thus expand_expr_real_1. */
promoted_nominal_mode
= promote_function_mode (data-nominal_type, data-nominal_mode,
unsignedp,
TREE_TYPE (current_function_decl), 2);
where nominal_type and nominal_mode are set up with TREE_TYPE (parm)
and TYPE_MODE (nominal_type). TREE_TYPE here is
I think the bug is here, not in combine.c. Can you try going back in history
for both snippets and see if they matched at some point?
The bug was introduced by
https://gcc.gnu.org/ml/gcc-cvs/2007-09/msg00613.html
commit 5d93234932c3d8617ce92b77b7013ef6bede9508
Author: shinwell shinwell@138bc75d-0d04-0410-961f-82ee72b054a4
Date: Thu Sep 20 11:01:18 2007 +
gcc/
* combine.c: Include cgraph.h.
(setup_incoming_promotions): Rework to allow more aggressive
elimination of sign extensions when all call sites of the
current function are known to lie within the current unit.
git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@128618
138bc75d-0d04-0410-961f-82ee72b054a4
Before this commit, combine.c has
enum machine_mode mode = TYPE_MODE (TREE_TYPE (arg));
int uns = TYPE_UNSIGNED (TREE_TYPE (arg));
mode = promote_mode (TREE_TYPE (arg), mode, uns, 1);
if (mode == GET_MODE (reg) mode != DECL_MODE (arg))
{
rtx x;
x = gen_rtx_CLOBBER (DECL_MODE (arg), const0_rtx);
x = gen_rtx_fmt_e ((uns ? ZERO_EXTEND : SIGN_EXTEND), mode, x);
record_value_for_reg (reg, first, x);
}
It matches function.c:
/* This is not really promoting for a call. However we need to be
consistent with assign_parm_find_data_types and expand_expr_real_1. */
promoted_nominal_mode
= promote_mode (data-nominal_type, data-nominal_mode, unsignedp, 1);
r128618 changed
mode = promote_mode (TREE_TYPE (arg), mode, uns, 1);
to
mode3 = promote_mode (DECL_ARG_TYPE (arg), mode2, uns3, 1);
It breaks none WORD_REGISTER_OPERATIONS targets.
Hmm, I think that DECL_ARG_TYPE makes a difference only
for non-WORD_REGISTER_OPERATIONS targets.
But yeah, isolated the above change looks wrong.
Your patch is ok for trunk if nobody objects within 24h and for branches
after a week.
Thanks,
Richard.
--
H.J.
PATCH: PR rtl-optimization/64037: Miscompilation with -Os and enum class : char parameter
Hi,
The enclosed testcase fails on x86 when compiled with -Os since we pass
a byte parameter with a byte load in caller and read it as an int in
callee. The reason it only shows up with -Os is x86 backend encodes
a byte load with an int load if -O isn't used. When a byte load is
used, the upper 24 bits of the register have random value for none
WORD_REGISTER_OPERATIONS targets.
It happens because setup_incoming_promotions in combine.c has
/* The mode and signedness of the argument before any promotions happen
(equal to the mode of the pseudo holding it at that stage). */
mode1 = TYPE_MODE (TREE_TYPE (arg));
uns1 = TYPE_UNSIGNED (TREE_TYPE (arg));
/* The mode and signedness of the argument after any source language and
TARGET_PROMOTE_PROTOTYPES-driven promotions. */
mode2 = TYPE_MODE (DECL_ARG_TYPE (arg));
uns3 = TYPE_UNSIGNED (DECL_ARG_TYPE (arg));
/* The mode and signedness of the argument as it is actually passed,
after any TARGET_PROMOTE_FUNCTION_ARGS-driven ABI promotions. */
mode3 = promote_function_mode (DECL_ARG_TYPE (arg), mode2, uns3,
TREE_TYPE (cfun-decl), 0);
while they are actually passed in register by assign_parm_setup_reg in
function.c:
/* Store the parm in a pseudoregister during the function, but we may
need to do it in a wider mode. Using 2 here makes the result
consistent with promote_decl_mode and thus expand_expr_real_1. */
promoted_nominal_mode
= promote_function_mode (data-nominal_type, data-nominal_mode, unsignedp,
TREE_TYPE (current_function_decl), 2);
where nominal_type and nominal_mode are set up with TREE_TYPE (parm)
and TYPE_MODE (nominal_type). TREE_TYPE here is
(gdb) call debug_tree (type)
enumeral_type 0x719f85e8 X
type integer_type 0x718a93f0 unsigned char public unsigned
string-flag QI
size integer_cst 0x718a5fa8 constant 8
unit size integer_cst 0x718a5fc0 constant 1
align 8 symtab 0 alias set -1 canonical type 0x718a93f0
precision 8 min integer_cst 0x718a5fd8 0 max integer_cst
0x718a5f78 255
static unsigned type_5 QI size integer_cst 0x718a5fa8 8 unit
size integer_cst 0x718a5fc0 1
align 8 symtab 0 alias set -1 canonical type 0x719f85e8
precision 8 min integer_cst 0x718a5fd8 0 max integer_cst
0x718a5f78 255
values tree_list 0x719fb028
purpose identifier_node 0x719f6738 V
bindings (nil)
local bindings (nil)
value const_decl 0x718c21c0 V type enumeral_type
0x719f85e8 X
readonly constant used VOID file pr64037.ii line 2 col 3
align 1 context enumeral_type 0x719f85e8 X initial
integer_cst 0x719d8d08 2 context translation_unit_decl
0x77ff91e0 D.1
chain type_decl 0x719f5c78 X
(gdb)
and DECL_ARG_TYPE is
(gdb) call debug_tree (type)
integer_type 0x718a9690 int public SI
size integer_cst 0x718a5e70 type integer_type 0x718a9150
bitsizetype constant 32
unit size integer_cst 0x718a5e88 type integer_type
0x718a90a8 sizetype constant 4
align 32 symtab 0 alias set 1 canonical type 0x718a9690
precision 32 min integer_cst 0x718c60c0 -2147483648 max
integer_cst 0x718c60d8 2147483647
pointer_to_this pointer_type 0x718cb930
(gdb)
This mismatch makes combine thinks a byte parameter is passed as int
in register and turns
(insn 9 6 10 2 (set (reg:SI 92 [ b ])
(zero_extend:SI (subreg:QI (reg:SI 91 [ b ]) 0))) pr64037.ii:9
138 {*zero_extendqisi2}
(expr_list:REG_DEAD (reg:SI 91 [ b ])
(nil)))
(insn 10 9 0 2 (set (mem:SI (reg/v/f:SI 88 [ out ]) [1 *out_4(D)+0 S4
A32])
(reg:SI 92 [ b ])) pr64037.ii:9 90 {*movsi_internal}
(expr_list:REG_DEAD (reg:SI 92 [ b ])
(expr_list:REG_DEAD (reg/v/f:SI 88 [ out ])
(nil
into
Trying 9 - 10:
Successfully matched this instruction:
(set (mem:SI (reg/v/f:SI 88 [ out ]) [1 *out_4(D)+0 S4 A32])
(reg:SI 91 [ b ]))
allowing combination of insns 9 and 10
original costs 6 + 4 = 10
replacement cost 4
deferring deletion of insn with uid = 9.
modifying insn i310: [r88:SI]=r91:SI
REG_DEAD r91:SI
REG_DEAD r88:SI
This patch makes setup_incoming_promotions to match assign_parm_setup_reg.
Tested on Linux/x86-64 without regressions. OK for trunk and backport?
Thanks.
H.J.
diff --git a/gcc/combine.c b/gcc/combine.c
index 1808f97..a0449a2 100644
--- a/gcc/combine.c
+++ b/gcc/combine.c
@@ -1561,8 +1561,8 @@ setup_incoming_promotions (rtx_insn *first)
uns3 = TYPE_UNSIGNED (DECL_ARG_TYPE (arg));
/* The mode and signedness of the argument as it is actually passed,
- after any TARGET_PROMOTE_FUNCTION_ARGS-driven ABI promotions. */
- mode3 = promote_function_mode (DECL_ARG_TYPE (arg), mode2, uns3,
+ see assign_parm_setup_reg in function.c. */
+
Re: PATCH: PR rtl-optimization/64037: Miscompilation with -Os and enum class : char parameter
On Tue, Nov 25, 2014 at 1:57 PM, H.J. Lu hongjiu...@intel.com wrote:
Hi,
The enclosed testcase fails on x86 when compiled with -Os since we pass
a byte parameter with a byte load in caller and read it as an int in
callee. The reason it only shows up with -Os is x86 backend encodes
a byte load with an int load if -O isn't used. When a byte load is
used, the upper 24 bits of the register have random value for none
WORD_REGISTER_OPERATIONS targets.
It happens because setup_incoming_promotions in combine.c has
/* The mode and signedness of the argument before any promotions happen
(equal to the mode of the pseudo holding it at that stage). */
mode1 = TYPE_MODE (TREE_TYPE (arg));
uns1 = TYPE_UNSIGNED (TREE_TYPE (arg));
/* The mode and signedness of the argument after any source language and
TARGET_PROMOTE_PROTOTYPES-driven promotions. */
mode2 = TYPE_MODE (DECL_ARG_TYPE (arg));
uns3 = TYPE_UNSIGNED (DECL_ARG_TYPE (arg));
/* The mode and signedness of the argument as it is actually passed,
after any TARGET_PROMOTE_FUNCTION_ARGS-driven ABI promotions. */
mode3 = promote_function_mode (DECL_ARG_TYPE (arg), mode2, uns3,
TREE_TYPE (cfun-decl), 0);
while they are actually passed in register by assign_parm_setup_reg in
function.c:
/* Store the parm in a pseudoregister during the function, but we may
need to do it in a wider mode. Using 2 here makes the result
consistent with promote_decl_mode and thus expand_expr_real_1. */
promoted_nominal_mode
= promote_function_mode (data-nominal_type, data-nominal_mode,
unsignedp,
TREE_TYPE (current_function_decl), 2);
where nominal_type and nominal_mode are set up with TREE_TYPE (parm)
and TYPE_MODE (nominal_type). TREE_TYPE here is
I think the bug is here, not in combine.c. Can you try going back in history
for both snippets and see if they matched at some point?
Thanks,
Richard.
(gdb) call debug_tree (type)
enumeral_type 0x719f85e8 X
type integer_type 0x718a93f0 unsigned char public unsigned
string-flag QI
size integer_cst 0x718a5fa8 constant 8
unit size integer_cst 0x718a5fc0 constant 1
align 8 symtab 0 alias set -1 canonical type 0x718a93f0
precision 8 min integer_cst 0x718a5fd8 0 max integer_cst
0x718a5f78 255
static unsigned type_5 QI size integer_cst 0x718a5fa8 8 unit
size integer_cst 0x718a5fc0 1
align 8 symtab 0 alias set -1 canonical type 0x719f85e8
precision 8 min integer_cst 0x718a5fd8 0 max integer_cst
0x718a5f78 255
values tree_list 0x719fb028
purpose identifier_node 0x719f6738 V
bindings (nil)
local bindings (nil)
value const_decl 0x718c21c0 V type enumeral_type
0x719f85e8 X
readonly constant used VOID file pr64037.ii line 2 col 3
align 1 context enumeral_type 0x719f85e8 X initial
integer_cst 0x719d8d08 2 context translation_unit_decl
0x77ff91e0 D.1
chain type_decl 0x719f5c78 X
(gdb)
and DECL_ARG_TYPE is
(gdb) call debug_tree (type)
integer_type 0x718a9690 int public SI
size integer_cst 0x718a5e70 type integer_type 0x718a9150
bitsizetype constant 32
unit size integer_cst 0x718a5e88 type integer_type
0x718a90a8 sizetype constant 4
align 32 symtab 0 alias set 1 canonical type 0x718a9690
precision 32 min integer_cst 0x718c60c0 -2147483648 max
integer_cst 0x718c60d8 2147483647
pointer_to_this pointer_type 0x718cb930
(gdb)
This mismatch makes combine thinks a byte parameter is passed as int
in register and turns
(insn 9 6 10 2 (set (reg:SI 92 [ b ])
(zero_extend:SI (subreg:QI (reg:SI 91 [ b ]) 0))) pr64037.ii:9
138 {*zero_extendqisi2}
(expr_list:REG_DEAD (reg:SI 91 [ b ])
(nil)))
(insn 10 9 0 2 (set (mem:SI (reg/v/f:SI 88 [ out ]) [1 *out_4(D)+0 S4
A32])
(reg:SI 92 [ b ])) pr64037.ii:9 90 {*movsi_internal}
(expr_list:REG_DEAD (reg:SI 92 [ b ])
(expr_list:REG_DEAD (reg/v/f:SI 88 [ out ])
(nil
into
Trying 9 - 10:
Successfully matched this instruction:
(set (mem:SI (reg/v/f:SI 88 [ out ]) [1 *out_4(D)+0 S4 A32])
(reg:SI 91 [ b ]))
allowing combination of insns 9 and 10
original costs 6 + 4 = 10
replacement cost 4
deferring deletion of insn with uid = 9.
modifying insn i310: [r88:SI]=r91:SI
REG_DEAD r91:SI
REG_DEAD r88:SI
This patch makes setup_incoming_promotions to match assign_parm_setup_reg.
Tested on Linux/x86-64 without regressions. OK for trunk and backport?
Thanks.
H.J.
diff --git a/gcc/combine.c b/gcc/combine.c
index 1808f97..a0449a2 100644
--- a/gcc/combine.c
+++ b/gcc/combine.c
@@ -1561,8 +1561,8 @@ setup_incoming_promotions (rtx_insn *first)
Re: PATCH: PR rtl-optimization/64037: Miscompilation with -Os and enum class : char parameter
On Tue, Nov 25, 2014 at 4:01 PM, Richard Biener
richard.guent...@gmail.com wrote:
On Tue, Nov 25, 2014 at 1:57 PM, H.J. Lu hongjiu...@intel.com wrote:
Hi,
The enclosed testcase fails on x86 when compiled with -Os since we pass
a byte parameter with a byte load in caller and read it as an int in
callee. The reason it only shows up with -Os is x86 backend encodes
a byte load with an int load if -O isn't used. When a byte load is
used, the upper 24 bits of the register have random value for none
WORD_REGISTER_OPERATIONS targets.
It happens because setup_incoming_promotions in combine.c has
/* The mode and signedness of the argument before any promotions happen
(equal to the mode of the pseudo holding it at that stage). */
mode1 = TYPE_MODE (TREE_TYPE (arg));
uns1 = TYPE_UNSIGNED (TREE_TYPE (arg));
/* The mode and signedness of the argument after any source language
and
TARGET_PROMOTE_PROTOTYPES-driven promotions. */
mode2 = TYPE_MODE (DECL_ARG_TYPE (arg));
uns3 = TYPE_UNSIGNED (DECL_ARG_TYPE (arg));
/* The mode and signedness of the argument as it is actually passed,
after any TARGET_PROMOTE_FUNCTION_ARGS-driven ABI promotions. */
mode3 = promote_function_mode (DECL_ARG_TYPE (arg), mode2, uns3,
TREE_TYPE (cfun-decl), 0);
while they are actually passed in register by assign_parm_setup_reg in
function.c:
/* Store the parm in a pseudoregister during the function, but we may
need to do it in a wider mode. Using 2 here makes the result
consistent with promote_decl_mode and thus expand_expr_real_1. */
promoted_nominal_mode
= promote_function_mode (data-nominal_type, data-nominal_mode,
unsignedp,
TREE_TYPE (current_function_decl), 2);
where nominal_type and nominal_mode are set up with TREE_TYPE (parm)
and TYPE_MODE (nominal_type). TREE_TYPE here is
I think the bug is here, not in combine.c. Can you try going back in history
for both snippets and see if they matched at some point?
Oh, and note that I think DECL_ARG_TYPE is sth dangerous - it's meant
to be a source language ABI kind-of-thing. Or rather an optimization
hit. For example in C when integral promotions happen to call arguments
this can be used to optimize sign-/zero-extensions in the callee. Unless
something else overrides this (like the target which specifies the real ABI).
IIRC.
Richard.
Thanks,
Richard.
(gdb) call debug_tree (type)
enumeral_type 0x719f85e8 X
type integer_type 0x718a93f0 unsigned char public unsigned
string-flag QI
size integer_cst 0x718a5fa8 constant 8
unit size integer_cst 0x718a5fc0 constant 1
align 8 symtab 0 alias set -1 canonical type 0x718a93f0
precision 8 min integer_cst 0x718a5fd8 0 max integer_cst
0x718a5f78 255
static unsigned type_5 QI size integer_cst 0x718a5fa8 8 unit
size integer_cst 0x718a5fc0 1
align 8 symtab 0 alias set -1 canonical type 0x719f85e8
precision 8 min integer_cst 0x718a5fd8 0 max integer_cst
0x718a5f78 255
values tree_list 0x719fb028
purpose identifier_node 0x719f6738 V
bindings (nil)
local bindings (nil)
value const_decl 0x718c21c0 V type enumeral_type
0x719f85e8 X
readonly constant used VOID file pr64037.ii line 2 col 3
align 1 context enumeral_type 0x719f85e8 X initial
integer_cst 0x719d8d08 2 context translation_unit_decl
0x77ff91e0 D.1
chain type_decl 0x719f5c78 X
(gdb)
and DECL_ARG_TYPE is
(gdb) call debug_tree (type)
integer_type 0x718a9690 int public SI
size integer_cst 0x718a5e70 type integer_type 0x718a9150
bitsizetype constant 32
unit size integer_cst 0x718a5e88 type integer_type
0x718a90a8 sizetype constant 4
align 32 symtab 0 alias set 1 canonical type 0x718a9690
precision 32 min integer_cst 0x718c60c0 -2147483648 max
integer_cst 0x718c60d8 2147483647
pointer_to_this pointer_type 0x718cb930
(gdb)
This mismatch makes combine thinks a byte parameter is passed as int
in register and turns
(insn 9 6 10 2 (set (reg:SI 92 [ b ])
(zero_extend:SI (subreg:QI (reg:SI 91 [ b ]) 0))) pr64037.ii:9
138 {*zero_extendqisi2}
(expr_list:REG_DEAD (reg:SI 91 [ b ])
(nil)))
(insn 10 9 0 2 (set (mem:SI (reg/v/f:SI 88 [ out ]) [1 *out_4(D)+0 S4
A32])
(reg:SI 92 [ b ])) pr64037.ii:9 90 {*movsi_internal}
(expr_list:REG_DEAD (reg:SI 92 [ b ])
(expr_list:REG_DEAD (reg/v/f:SI 88 [ out ])
(nil
into
Trying 9 - 10:
Successfully matched this instruction:
(set (mem:SI (reg/v/f:SI 88 [ out ]) [1 *out_4(D)+0 S4 A32])
(reg:SI 91 [ b ]))
allowing combination of insns 9 and 10
original costs 6 + 4 = 10
replacement cost 4
deferring deletion of insn
Re: PATCH: PR rtl-optimization/64037: Miscompilation with -Os and enum class : char parameter
On Tue, Nov 25, 2014 at 7:01 AM, Richard Biener
richard.guent...@gmail.com wrote:
On Tue, Nov 25, 2014 at 1:57 PM, H.J. Lu hongjiu...@intel.com wrote:
Hi,
The enclosed testcase fails on x86 when compiled with -Os since we pass
a byte parameter with a byte load in caller and read it as an int in
callee. The reason it only shows up with -Os is x86 backend encodes
a byte load with an int load if -O isn't used. When a byte load is
used, the upper 24 bits of the register have random value for none
WORD_REGISTER_OPERATIONS targets.
It happens because setup_incoming_promotions in combine.c has
/* The mode and signedness of the argument before any promotions happen
(equal to the mode of the pseudo holding it at that stage). */
mode1 = TYPE_MODE (TREE_TYPE (arg));
uns1 = TYPE_UNSIGNED (TREE_TYPE (arg));
/* The mode and signedness of the argument after any source language
and
TARGET_PROMOTE_PROTOTYPES-driven promotions. */
mode2 = TYPE_MODE (DECL_ARG_TYPE (arg));
uns3 = TYPE_UNSIGNED (DECL_ARG_TYPE (arg));
/* The mode and signedness of the argument as it is actually passed,
after any TARGET_PROMOTE_FUNCTION_ARGS-driven ABI promotions. */
mode3 = promote_function_mode (DECL_ARG_TYPE (arg), mode2, uns3,
TREE_TYPE (cfun-decl), 0);
while they are actually passed in register by assign_parm_setup_reg in
function.c:
/* Store the parm in a pseudoregister during the function, but we may
need to do it in a wider mode. Using 2 here makes the result
consistent with promote_decl_mode and thus expand_expr_real_1. */
promoted_nominal_mode
= promote_function_mode (data-nominal_type, data-nominal_mode,
unsignedp,
TREE_TYPE (current_function_decl), 2);
where nominal_type and nominal_mode are set up with TREE_TYPE (parm)
and TYPE_MODE (nominal_type). TREE_TYPE here is
I think the bug is here, not in combine.c. Can you try going back in history
for both snippets and see if they matched at some point?
The bug was introduced by
https://gcc.gnu.org/ml/gcc-cvs/2007-09/msg00613.html
commit 5d93234932c3d8617ce92b77b7013ef6bede9508
Author: shinwell shinwell@138bc75d-0d04-0410-961f-82ee72b054a4
Date: Thu Sep 20 11:01:18 2007 +
gcc/
* combine.c: Include cgraph.h.
(setup_incoming_promotions): Rework to allow more aggressive
elimination of sign extensions when all call sites of the
current function are known to lie within the current unit.
git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@128618
138bc75d-0d04-0410-961f-82ee72b054a4
Before this commit, combine.c has
enum machine_mode mode = TYPE_MODE (TREE_TYPE (arg));
int uns = TYPE_UNSIGNED (TREE_TYPE (arg));
mode = promote_mode (TREE_TYPE (arg), mode, uns, 1);
if (mode == GET_MODE (reg) mode != DECL_MODE (arg))
{
rtx x;
x = gen_rtx_CLOBBER (DECL_MODE (arg), const0_rtx);
x = gen_rtx_fmt_e ((uns ? ZERO_EXTEND : SIGN_EXTEND), mode, x);
record_value_for_reg (reg, first, x);
}
It matches function.c:
/* This is not really promoting for a call. However we need to be
consistent with assign_parm_find_data_types and expand_expr_real_1. */
promoted_nominal_mode
= promote_mode (data-nominal_type, data-nominal_mode, unsignedp, 1);
r128618 changed
mode = promote_mode (TREE_TYPE (arg), mode, uns, 1);
to
mode3 = promote_mode (DECL_ARG_TYPE (arg), mode2, uns3, 1);
It breaks none WORD_REGISTER_OPERATIONS targets.
--
H.J.
Re: PATCH: PR rtl-optimization/64037: Miscompilation with -Os and enum class : char parameter
On Tue, Nov 25, 2014 at 7:04 AM, Richard Biener
richard.guent...@gmail.com wrote:
On Tue, Nov 25, 2014 at 4:01 PM, Richard Biener
richard.guent...@gmail.com wrote:
On Tue, Nov 25, 2014 at 1:57 PM, H.J. Lu hongjiu...@intel.com wrote:
Hi,
The enclosed testcase fails on x86 when compiled with -Os since we pass
a byte parameter with a byte load in caller and read it as an int in
callee. The reason it only shows up with -Os is x86 backend encodes
a byte load with an int load if -O isn't used. When a byte load is
used, the upper 24 bits of the register have random value for none
WORD_REGISTER_OPERATIONS targets.
It happens because setup_incoming_promotions in combine.c has
/* The mode and signedness of the argument before any promotions
happen
(equal to the mode of the pseudo holding it at that stage). */
mode1 = TYPE_MODE (TREE_TYPE (arg));
uns1 = TYPE_UNSIGNED (TREE_TYPE (arg));
/* The mode and signedness of the argument after any source language
and
TARGET_PROMOTE_PROTOTYPES-driven promotions. */
mode2 = TYPE_MODE (DECL_ARG_TYPE (arg));
uns3 = TYPE_UNSIGNED (DECL_ARG_TYPE (arg));
/* The mode and signedness of the argument as it is actually passed,
after any TARGET_PROMOTE_FUNCTION_ARGS-driven ABI promotions. */
mode3 = promote_function_mode (DECL_ARG_TYPE (arg), mode2, uns3,
TREE_TYPE (cfun-decl), 0);
while they are actually passed in register by assign_parm_setup_reg in
function.c:
/* Store the parm in a pseudoregister during the function, but we may
need to do it in a wider mode. Using 2 here makes the result
consistent with promote_decl_mode and thus expand_expr_real_1. */
promoted_nominal_mode
= promote_function_mode (data-nominal_type, data-nominal_mode,
unsignedp,
TREE_TYPE (current_function_decl), 2);
where nominal_type and nominal_mode are set up with TREE_TYPE (parm)
and TYPE_MODE (nominal_type). TREE_TYPE here is
I think the bug is here, not in combine.c. Can you try going back in history
for both snippets and see if they matched at some point?
Oh, and note that I think DECL_ARG_TYPE is sth dangerous - it's meant
to be a source language ABI kind-of-thing. Or rather an optimization
hit. For example in C when integral promotions happen to call arguments
this can be used to optimize sign-/zero-extensions in the callee. Unless
something else overrides this (like the target which specifies the real ABI).
IIRC.
PROMOTE_MODE is a performance hint, not an ABI requirement.
i386.h has
#define PROMOTE_MODE(MODE, UNSIGNEDP, TYPE) \
do {\
if (((MODE) == HImode TARGET_PROMOTE_HI_REGS) \
|| ((MODE) == QImode TARGET_PROMOTE_QI_REGS)) \
(MODE) = SImode;\
} while (0)
We may promote QI/HI to SI, depending on optimization.
On the other hand, TARGET_PROMOTE_FUNCTION_MODE is
determined by psABI.
I am enclosing the missing ChangeLog entries.
--
H.J.
---
gcc/
PR rtl-optimization/64037
* combine.c (setup_incoming_promotions): Pass the argument
before any promotions happen to promote_function_mode.
gcc/testsuite/
PR rtl-optimization/64037
* g++.dg/pr64037.C: New test.
