On 30/05/15 14:54, Jeff Law wrote:
> On 05/29/2015 12:32 AM, Kugan wrote:
>>>>>
>>>>> PR target/65768
>>>>> * cprop.c (try_replace_reg): Check cost of constants before
>>>>> propagating.
>>> I should have also noted, fresh bootstrap & regression test is needed
>>> too.
>>
>> Thanks Jeff for the comments. I did a fresh bootstrap and regression
>> testing on x86_64-linux-gnu with no new regression. I will wait for
>> you ACK.
> Can you address the 3 issues in my prior message? I'll include them
> here for clarity:
>
> --
>
> The "const_p" variable is poorly named, though I can kindof see how you
> settled on it. Maybe "check_rtx_costs" or something along those lines
> would be better.
>
> The comment for the second hunk would probably be better as:
>
> /* If TO is a constant, check the cost of the set after propagation
> to the cost of the set before the propagation. If the cost is
> higher, then do not replace FROM with TO. */
>
>
> You should try to produce a testcase where this change shows a code
> generation improvement. Given we're checking target costs, that test
> will naturally be target specific. But please do try.
>
> So with the two nits fixed and a testcase, I think this can go forward.
> --
>
Thanks Jeff and apologies for missing your previous email. I have now
fixed the comments as you suggested and changed the PR target/65768
testcase such that it tests this case.
I will commit it if there is no objections to this.
Thanks,
Kugan
gcc/ChangeLog:
2015-06-01 Kugan Vivekanandarajah <kug...@linaro.org>
Zhenqiang Chen <zhenqiang.c...@linaro.org>
PR target/65768
* cprop.c (try_replace_reg): Check cost of constants before propagating.
gcc/testsuite/ChangeLog:
2015-06-01 Kugan Vivekanandarajah <kug...@linaro.org>
PR target/65768
* gcc.target/arm/maskdata.c: Remove -fno-gcse.
diff --git a/gcc/cprop.c b/gcc/cprop.c
index 57c44ef..94bb064 100644
--- a/gcc/cprop.c
+++ b/gcc/cprop.c
@@ -765,12 +765,37 @@ try_replace_reg (rtx from, rtx to, rtx_insn *insn)
int success = 0;
rtx set = single_set (insn);
+ bool check_rtx_costs = true;
+ bool speed = optimize_bb_for_speed_p (BLOCK_FOR_INSN (insn));
+ int old_cost = set ? set_rtx_cost (set, speed) : 0;
+
+ if ((note != 0
+ && REG_NOTE_KIND (note) == REG_EQUAL
+ && (GET_CODE (XEXP (note, 0)) == CONST
+ || CONSTANT_P (XEXP (note, 0))))
+ || (set && CONSTANT_P (SET_SRC (set))))
+ check_rtx_costs = false;
+
/* Usually we substitute easy stuff, so we won't copy everything.
We however need to take care to not duplicate non-trivial CONST
expressions. */
to = copy_rtx (to);
validate_replace_src_group (from, to, insn);
+
+ /* If TO is a constant, check the cost of the set after propagation
+ to the cost of the set before the propagation. If the cost is
+ higher, then do not replace FROM with TO. */
+
+ if (check_rtx_costs
+ && CONSTANT_P (to)
+ && (set_rtx_cost (set, speed) > old_cost))
+ {
+ cancel_changes (0);
+ return false;
+ }
+
+
if (num_changes_pending () && apply_change_group ())
success = 1;
diff --git a/gcc/testsuite/gcc.target/arm/maskdata.c
b/gcc/testsuite/gcc.target/arm/maskdata.c
index 6d6bb39..35d2f06 100644
--- a/gcc/testsuite/gcc.target/arm/maskdata.c
+++ b/gcc/testsuite/gcc.target/arm/maskdata.c
@@ -1,5 +1,5 @@
/* { dg-do compile } */
-/* { dg-options " -O2 -fno-gcse " } */
+/* { dg-options " -O2" } */
/* { dg-require-effective-target arm_thumb2_ok } */
#define MASK 0xff00ff
