Re: [PATCH, PR target/65103, 2/3] Propagate address constants into loops for i386

2015-07-09 Thread Jeff Law

On 06/01/2015 06:26 AM, Ilya Enkovich wrote:

2015-05-29 1:15 GMT+03:00 Jeff Law l...@redhat.com:


Right, but you're blindly propagating.  The right thing to do is look at
some kind of metric to estimate when it's profitable to propagate the
constant back in vs leave it hoisted out.


No, the patch is not to blindly propagate but to let loop invariant to
be propagated into address. Existing propagation gain estimation
(should_replace_address) still applies.
Agreed.  I missed the costing metric in should_replace_address.  Sorry 
for not looking more deeply at that and delaying progress on this issue.


So looking more closely at the patch itself...

AFAICT, your change to that condition potentially allows fwprop to 
propagate any constants to any use, regardless of context (mem vs non-mem).


You're largely getting away with that because you're checking for the 
rtx code CONST -- which filters out everything except symbolic 
constants.  ie, you're filtering out CONST_INT, CONST_DOUBLE, CONST_*.


So the first thing you need to do is clarify that comment a bit.   Perhaps:

/* Do not propagate loop invariant definitions into a different loop,
   except for symbolic constants which may propagate, subject to cost
   testing.  */


The second issue is I think the testcase has been compromised.  If I 
compile the test on the trunk, I don't see any references to GOTOFF.  So 
I think you need to create a new testcase.  One way would be to 
instrument a build, detecting anytime you were able to propagate a 
symbolic constant into a different loop nest -- if you can discover one, 
then use creduce or multidelta to produce a simplified testcase.


Because of the CONST vs CONST_INT stuff, you're not running the risk of 
regressing 65768, so no need to build an arm cross a test that :-)


With the comment update and a new testcase this is OK for the trunk.

Again, sorry for delaying progress on this.

Jeff


Re: [PATCH, PR target/65103, 2/3] Propagate address constants into loops for i386

2015-06-25 Thread Ilya Enkovich
Ping

2015-06-01 15:26 GMT+03:00 Ilya Enkovich enkovich@gmail.com:
 2015-05-29 1:15 GMT+03:00 Jeff Law l...@redhat.com:

 Right, but you're blindly propagating.  The right thing to do is look at
 some kind of metric to estimate when it's profitable to propagate the
 constant back in vs leave it hoisted out.

 No, the patch is not to blindly propagate but to let loop invariant to
 be propagated into address. Existing propagation gain estimation
 (should_replace_address) still applies.

 Thanks,
 Ilya


 If you look at what Kugan is doing in cprop.c, that's exactly the approach
 he's taking -- looking at rtx costing to determine when to propagate the
 constant back into the loop.  It could probably be made better with some
 knowledge of register pressure and looking at whether or not all uses (vs
 just some uses) of the constant will be propagated.

 Jeff


Re: [PATCH, PR target/65103, 2/3] Propagate address constants into loops for i386

2015-06-01 Thread Ilya Enkovich
2015-05-29 1:15 GMT+03:00 Jeff Law l...@redhat.com:

 Right, but you're blindly propagating.  The right thing to do is look at
 some kind of metric to estimate when it's profitable to propagate the
 constant back in vs leave it hoisted out.

No, the patch is not to blindly propagate but to let loop invariant to
be propagated into address. Existing propagation gain estimation
(should_replace_address) still applies.

Thanks,
Ilya


 If you look at what Kugan is doing in cprop.c, that's exactly the approach
 he's taking -- looking at rtx costing to determine when to propagate the
 constant back into the loop.  It could probably be made better with some
 knowledge of register pressure and looking at whether or not all uses (vs
 just some uses) of the constant will be propagated.

 Jeff


Re: [PATCH, PR target/65103, 2/3] Propagate address constants into loops for i386

2015-05-28 Thread Jeff Law

On 05/05/2015 05:05 AM, Ilya Enkovich wrote:

2015-04-21 8:52 GMT+03:00 Jeff Law l...@redhat.com:

On 04/17/2015 02:34 AM, Ilya Enkovich wrote:


On 15 Apr 14:07, Ilya Enkovich wrote:


2015-04-14 8:22 GMT+03:00 Jeff Law l...@redhat.com:


On 03/15/2015 02:30 PM, Richard Sandiford wrote:



Ilya Enkovich enkovich@gmail.com writes:



This patch allows propagation of loop invariants for i386 if
propagated
value is a constant to be used in address operand.  Bootstrapped and
tested on x86_64-unknown-linux-gnu.  OK for trunk or stage 1?




Is it necessary for this to be a target hook?  The concept doesn't seem
particularly target-specific.  We should only propagate into the
address
if the new cost is no greater than the old cost, but if the address
meets that condition and if propagating at this point in the pipeline
is
a win on x86, then wouldn't it be a win for other targets too?



I agree with Richard here.  I can't see a strong reason why this should
be a
target hook.

Perhaps part of the issue here is the address costing metrics may not
have
enough context to make good decisions.  In which case what context do
they
need?



At this point I don't insist on a target hook.  The main reasoning was
to not affect other targets. If we extend propagation for non constant
values different aspects may appear. E.g. possible register pressure
changes may significantly affect ia32. I just wanted to have an
instrument to play with a propagation on x86 not affecting other
targets. I don't have an opportunity to test possible performance
implications on non-x86 targets. Don't expect (significant)
regressions there but who knows...

I'll remove the hook from this patch. Will probably introduce it later
if some target specific cases are found.

Thanks,
Ilya



Jeff



Here is a version with no hook.  Bootstrapped and tested on
x86_64-unknown-linux-gnu.  Is it OK for trunk?

Thanks,
Ilya
--
gcc/

2015-04-17  Ilya Enkovich  ilya.enkov...@intel.com

 PR target/65103
 * fwprop.c (forward_propagate_into): Propagate loop
 invariants if a target says so.

gcc/testsuite/

2015-04-17  Ilya Enkovich  ilya.enkov...@intel.com

 PR target/65103
 * gcc.target/i386/pr65103-2.c: New.


It seems to me there's a key piece missing here -- metrics.

When is this profitable, when is it not profitable.   Just blindly undoing
LICM seems wrong here.

The first thought is to look at register pressure through the loop.  I
thought we had some infrastructure for this kind of query available. It'd
probably be wise to re-use it.  In fact, one might reasonably ask if LICM
should have hoisted the expression to start with.


I'd also think the cost of the constant may come into play here.  A really
cheap constant probably should not have been hoisted by LICM to start with
-- but the code may have been written in such a way that some low cost
constants are pulled out as loop invariants at the source level.  So this
isn't strictly an issue of un-doing bad LICM

So I think to go forward we need to be working on solving the when is this
a profitable transformation to make.


This patch doesn't force propagation.  The patch just allows
propagation and regular fwprop cost estimation is used to compute if
this is profitable.  For i386 I don't see cases when we shouldn't
propagate. We remove instruction, reduce register pressure and having
constant in memory operand is free which is reflected in address_cost
hook.
Right, but you're blindly propagating.  The right thing to do is look at 
some kind of metric to estimate when it's profitable to propagate the 
constant back in vs leave it hoisted out.


If you look at what Kugan is doing in cprop.c, that's exactly the 
approach he's taking -- looking at rtx costing to determine when to 
propagate the constant back into the loop.  It could probably be made 
better with some knowledge of register pressure and looking at whether 
or not all uses (vs just some uses) of the constant will be propagated.


Jeff


Re: [PATCH, PR target/65103, 2/3] Propagate address constants into loops for i386

2015-05-21 Thread Ilya Enkovich
Ping

2015-05-05 14:05 GMT+03:00 Ilya Enkovich enkovich@gmail.com:
 2015-04-21 8:52 GMT+03:00 Jeff Law l...@redhat.com:
 On 04/17/2015 02:34 AM, Ilya Enkovich wrote:

 On 15 Apr 14:07, Ilya Enkovich wrote:

 2015-04-14 8:22 GMT+03:00 Jeff Law l...@redhat.com:

 On 03/15/2015 02:30 PM, Richard Sandiford wrote:


 Ilya Enkovich enkovich@gmail.com writes:


 This patch allows propagation of loop invariants for i386 if
 propagated
 value is a constant to be used in address operand.  Bootstrapped and
 tested on x86_64-unknown-linux-gnu.  OK for trunk or stage 1?



 Is it necessary for this to be a target hook?  The concept doesn't seem
 particularly target-specific.  We should only propagate into the
 address
 if the new cost is no greater than the old cost, but if the address
 meets that condition and if propagating at this point in the pipeline
 is
 a win on x86, then wouldn't it be a win for other targets too?


 I agree with Richard here.  I can't see a strong reason why this should
 be a
 target hook.

 Perhaps part of the issue here is the address costing metrics may not
 have
 enough context to make good decisions.  In which case what context do
 they
 need?


 At this point I don't insist on a target hook.  The main reasoning was
 to not affect other targets. If we extend propagation for non constant
 values different aspects may appear. E.g. possible register pressure
 changes may significantly affect ia32. I just wanted to have an
 instrument to play with a propagation on x86 not affecting other
 targets. I don't have an opportunity to test possible performance
 implications on non-x86 targets. Don't expect (significant)
 regressions there but who knows...

 I'll remove the hook from this patch. Will probably introduce it later
 if some target specific cases are found.

 Thanks,
 Ilya


 Jeff


 Here is a version with no hook.  Bootstrapped and tested on
 x86_64-unknown-linux-gnu.  Is it OK for trunk?

 Thanks,
 Ilya
 --
 gcc/

 2015-04-17  Ilya Enkovich  ilya.enkov...@intel.com

 PR target/65103
 * fwprop.c (forward_propagate_into): Propagate loop
 invariants if a target says so.

 gcc/testsuite/

 2015-04-17  Ilya Enkovich  ilya.enkov...@intel.com

 PR target/65103
 * gcc.target/i386/pr65103-2.c: New.

 It seems to me there's a key piece missing here -- metrics.

 When is this profitable, when is it not profitable.   Just blindly undoing
 LICM seems wrong here.

 The first thought is to look at register pressure through the loop.  I
 thought we had some infrastructure for this kind of query available. It'd
 probably be wise to re-use it.  In fact, one might reasonably ask if LICM
 should have hoisted the expression to start with.


 I'd also think the cost of the constant may come into play here.  A really
 cheap constant probably should not have been hoisted by LICM to start with
 -- but the code may have been written in such a way that some low cost
 constants are pulled out as loop invariants at the source level.  So this
 isn't strictly an issue of un-doing bad LICM

 So I think to go forward we need to be working on solving the when is this
 a profitable transformation to make.

 This patch doesn't force propagation.  The patch just allows
 propagation and regular fwprop cost estimation is used to compute if
 this is profitable.  For i386 I don't see cases when we shouldn't
 propagate. We remove instruction, reduce register pressure and having
 constant in memory operand is free which is reflected in address_cost
 hook.

 Ilya


 jeff


Re: [PATCH, PR target/65103, 2/3] Propagate address constants into loops for i386

2015-05-05 Thread Ilya Enkovich
2015-04-21 8:52 GMT+03:00 Jeff Law l...@redhat.com:
 On 04/17/2015 02:34 AM, Ilya Enkovich wrote:

 On 15 Apr 14:07, Ilya Enkovich wrote:

 2015-04-14 8:22 GMT+03:00 Jeff Law l...@redhat.com:

 On 03/15/2015 02:30 PM, Richard Sandiford wrote:


 Ilya Enkovich enkovich@gmail.com writes:


 This patch allows propagation of loop invariants for i386 if
 propagated
 value is a constant to be used in address operand.  Bootstrapped and
 tested on x86_64-unknown-linux-gnu.  OK for trunk or stage 1?



 Is it necessary for this to be a target hook?  The concept doesn't seem
 particularly target-specific.  We should only propagate into the
 address
 if the new cost is no greater than the old cost, but if the address
 meets that condition and if propagating at this point in the pipeline
 is
 a win on x86, then wouldn't it be a win for other targets too?


 I agree with Richard here.  I can't see a strong reason why this should
 be a
 target hook.

 Perhaps part of the issue here is the address costing metrics may not
 have
 enough context to make good decisions.  In which case what context do
 they
 need?


 At this point I don't insist on a target hook.  The main reasoning was
 to not affect other targets. If we extend propagation for non constant
 values different aspects may appear. E.g. possible register pressure
 changes may significantly affect ia32. I just wanted to have an
 instrument to play with a propagation on x86 not affecting other
 targets. I don't have an opportunity to test possible performance
 implications on non-x86 targets. Don't expect (significant)
 regressions there but who knows...

 I'll remove the hook from this patch. Will probably introduce it later
 if some target specific cases are found.

 Thanks,
 Ilya


 Jeff


 Here is a version with no hook.  Bootstrapped and tested on
 x86_64-unknown-linux-gnu.  Is it OK for trunk?

 Thanks,
 Ilya
 --
 gcc/

 2015-04-17  Ilya Enkovich  ilya.enkov...@intel.com

 PR target/65103
 * fwprop.c (forward_propagate_into): Propagate loop
 invariants if a target says so.

 gcc/testsuite/

 2015-04-17  Ilya Enkovich  ilya.enkov...@intel.com

 PR target/65103
 * gcc.target/i386/pr65103-2.c: New.

 It seems to me there's a key piece missing here -- metrics.

 When is this profitable, when is it not profitable.   Just blindly undoing
 LICM seems wrong here.

 The first thought is to look at register pressure through the loop.  I
 thought we had some infrastructure for this kind of query available. It'd
 probably be wise to re-use it.  In fact, one might reasonably ask if LICM
 should have hoisted the expression to start with.


 I'd also think the cost of the constant may come into play here.  A really
 cheap constant probably should not have been hoisted by LICM to start with
 -- but the code may have been written in such a way that some low cost
 constants are pulled out as loop invariants at the source level.  So this
 isn't strictly an issue of un-doing bad LICM

 So I think to go forward we need to be working on solving the when is this
 a profitable transformation to make.

This patch doesn't force propagation.  The patch just allows
propagation and regular fwprop cost estimation is used to compute if
this is profitable.  For i386 I don't see cases when we shouldn't
propagate. We remove instruction, reduce register pressure and having
constant in memory operand is free which is reflected in address_cost
hook.

Ilya


 jeff


Re: [PATCH, PR target/65103, 2/3] Propagate address constants into loops for i386

2015-04-21 Thread Jeff Law

On 04/17/2015 02:34 AM, Ilya Enkovich wrote:

On 15 Apr 14:07, Ilya Enkovich wrote:

2015-04-14 8:22 GMT+03:00 Jeff Law l...@redhat.com:

On 03/15/2015 02:30 PM, Richard Sandiford wrote:


Ilya Enkovich enkovich@gmail.com writes:


This patch allows propagation of loop invariants for i386 if propagated
value is a constant to be used in address operand.  Bootstrapped and
tested on x86_64-unknown-linux-gnu.  OK for trunk or stage 1?



Is it necessary for this to be a target hook?  The concept doesn't seem
particularly target-specific.  We should only propagate into the address
if the new cost is no greater than the old cost, but if the address
meets that condition and if propagating at this point in the pipeline is
a win on x86, then wouldn't it be a win for other targets too?


I agree with Richard here.  I can't see a strong reason why this should be a
target hook.

Perhaps part of the issue here is the address costing metrics may not have
enough context to make good decisions.  In which case what context do they
need?


At this point I don't insist on a target hook.  The main reasoning was
to not affect other targets. If we extend propagation for non constant
values different aspects may appear. E.g. possible register pressure
changes may significantly affect ia32. I just wanted to have an
instrument to play with a propagation on x86 not affecting other
targets. I don't have an opportunity to test possible performance
implications on non-x86 targets. Don't expect (significant)
regressions there but who knows...

I'll remove the hook from this patch. Will probably introduce it later
if some target specific cases are found.

Thanks,
Ilya



Jeff


Here is a version with no hook.  Bootstrapped and tested on 
x86_64-unknown-linux-gnu.  Is it OK for trunk?

Thanks,
Ilya
--
gcc/

2015-04-17  Ilya Enkovich  ilya.enkov...@intel.com

PR target/65103
* fwprop.c (forward_propagate_into): Propagate loop
invariants if a target says so.

gcc/testsuite/

2015-04-17  Ilya Enkovich  ilya.enkov...@intel.com

PR target/65103
* gcc.target/i386/pr65103-2.c: New.

It seems to me there's a key piece missing here -- metrics.

When is this profitable, when is it not profitable.   Just blindly 
undoing LICM seems wrong here.


The first thought is to look at register pressure through the loop.  I 
thought we had some infrastructure for this kind of query available. 
It'd probably be wise to re-use it.  In fact, one might reasonably ask 
if LICM should have hoisted the expression to start with.



I'd also think the cost of the constant may come into play here.  A 
really cheap constant probably should not have been hoisted by LICM to 
start with -- but the code may have been written in such a way that some 
low cost constants are pulled out as loop invariants at the source 
level.  So this isn't strictly an issue of un-doing bad LICM


So I think to go forward we need to be working on solving the when is 
this a profitable transformation to make.


jeff


Re: [PATCH, PR target/65103, 2/3] Propagate address constants into loops for i386

2015-04-17 Thread Ilya Enkovich
On 15 Apr 14:07, Ilya Enkovich wrote:
 2015-04-14 8:22 GMT+03:00 Jeff Law l...@redhat.com:
  On 03/15/2015 02:30 PM, Richard Sandiford wrote:
 
  Ilya Enkovich enkovich@gmail.com writes:
 
  This patch allows propagation of loop invariants for i386 if propagated
  value is a constant to be used in address operand.  Bootstrapped and
  tested on x86_64-unknown-linux-gnu.  OK for trunk or stage 1?
 
 
  Is it necessary for this to be a target hook?  The concept doesn't seem
  particularly target-specific.  We should only propagate into the address
  if the new cost is no greater than the old cost, but if the address
  meets that condition and if propagating at this point in the pipeline is
  a win on x86, then wouldn't it be a win for other targets too?
 
  I agree with Richard here.  I can't see a strong reason why this should be a
  target hook.
 
  Perhaps part of the issue here is the address costing metrics may not have
  enough context to make good decisions.  In which case what context do they
  need?
 
 At this point I don't insist on a target hook.  The main reasoning was
 to not affect other targets. If we extend propagation for non constant
 values different aspects may appear. E.g. possible register pressure
 changes may significantly affect ia32. I just wanted to have an
 instrument to play with a propagation on x86 not affecting other
 targets. I don't have an opportunity to test possible performance
 implications on non-x86 targets. Don't expect (significant)
 regressions there but who knows...
 
 I'll remove the hook from this patch. Will probably introduce it later
 if some target specific cases are found.
 
 Thanks,
 Ilya
 
 
  Jeff

Here is a version with no hook.  Bootstrapped and tested on 
x86_64-unknown-linux-gnu.  Is it OK for trunk?

Thanks,
Ilya
--
gcc/

2015-04-17  Ilya Enkovich  ilya.enkov...@intel.com

PR target/65103
* fwprop.c (forward_propagate_into): Propagate loop
invariants if a target says so.

gcc/testsuite/

2015-04-17  Ilya Enkovich  ilya.enkov...@intel.com

PR target/65103
* gcc.target/i386/pr65103-2.c: New.


diff --git a/gcc/fwprop.c b/gcc/fwprop.c
index fc64ec9..82ebd01 100644
--- a/gcc/fwprop.c
+++ b/gcc/fwprop.c
@@ -1365,8 +1365,18 @@ forward_propagate_into (df_ref use)
   if (DF_REF_IS_ARTIFICIAL (def))
 return false;
 
-  /* Do not propagate loop invariant definitions inside the loop.  */
-  if (DF_REF_BB (def)-loop_father != DF_REF_BB (use)-loop_father)
+  def_insn = DF_REF_INSN (def);
+  if (multiple_sets (def_insn))
+return false;
+  def_set = single_set (def_insn);
+  if (!def_set)
+return false;
+
+  /* Do not propagate loop invariant definitions inside the loop.
+ Allow address constant propagation.  */
+  if (DF_REF_BB (def)-loop_father != DF_REF_BB (use)-loop_father
+   (DF_REF_TYPE (use) == DF_REF_REG_USE
+ || GET_CODE (SET_SRC (def_set)) != CONST))
 return false;
 
   /* Check if the use is still present in the insn!  */
@@ -1379,13 +1389,6 @@ forward_propagate_into (df_ref use)
   if (!reg_mentioned_p (DF_REF_REG (use), parent))
 return false;
 
-  def_insn = DF_REF_INSN (def);
-  if (multiple_sets (def_insn))
-return false;
-  def_set = single_set (def_insn);
-  if (!def_set)
-return false;
-
   /* Only try one kind of propagation.  If two are possible, we'll
  do it on the following iterations.  */
   if (forward_propagate_and_simplify (use, def_insn, def_set)
diff --git a/gcc/testsuite/gcc.target/i386/pr65103-2.c 
b/gcc/testsuite/gcc.target/i386/pr65103-2.c
new file mode 100644
index 000..b7a32f7
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr65103-2.c
@@ -0,0 +1,24 @@
+/* { dg-do compile { target ia32 } } */
+/* { dg-require-effective-target pie } */
+/* { dg-options -O2 -fPIE } */
+/* { dg-final { scan-assembler-not GOTOFF, } } */
+
+typedef struct S
+{
+  int a;
+  int b;
+} S;
+struct S gs;
+
+extern int compute ( struct S * );
+
+int test( void )
+{
+int t = -1;
+while (t)
+  {
+   gs.a++;
+   t = compute (gs);
+  }
+return 0;
+}


Re: [PATCH, PR target/65103, 2/3] Propagate address constants into loops for i386

2015-04-15 Thread Ilya Enkovich
2015-04-14 8:22 GMT+03:00 Jeff Law l...@redhat.com:
 On 03/15/2015 02:30 PM, Richard Sandiford wrote:

 Ilya Enkovich enkovich@gmail.com writes:

 This patch allows propagation of loop invariants for i386 if propagated
 value is a constant to be used in address operand.  Bootstrapped and
 tested on x86_64-unknown-linux-gnu.  OK for trunk or stage 1?


 Is it necessary for this to be a target hook?  The concept doesn't seem
 particularly target-specific.  We should only propagate into the address
 if the new cost is no greater than the old cost, but if the address
 meets that condition and if propagating at this point in the pipeline is
 a win on x86, then wouldn't it be a win for other targets too?

 I agree with Richard here.  I can't see a strong reason why this should be a
 target hook.

 Perhaps part of the issue here is the address costing metrics may not have
 enough context to make good decisions.  In which case what context do they
 need?

At this point I don't insist on a target hook.  The main reasoning was
to not affect other targets. If we extend propagation for non constant
values different aspects may appear. E.g. possible register pressure
changes may significantly affect ia32. I just wanted to have an
instrument to play with a propagation on x86 not affecting other
targets. I don't have an opportunity to test possible performance
implications on non-x86 targets. Don't expect (significant)
regressions there but who knows...

I'll remove the hook from this patch. Will probably introduce it later
if some target specific cases are found.

Thanks,
Ilya


 Jeff


Re: [PATCH, PR target/65103, 2/3] Propagate address constants into loops for i386

2015-04-13 Thread Jeff Law

On 03/15/2015 02:30 PM, Richard Sandiford wrote:

Ilya Enkovich enkovich@gmail.com writes:

This patch allows propagation of loop invariants for i386 if propagated
value is a constant to be used in address operand.  Bootstrapped and
tested on x86_64-unknown-linux-gnu.  OK for trunk or stage 1?


Is it necessary for this to be a target hook?  The concept doesn't seem
particularly target-specific.  We should only propagate into the address
if the new cost is no greater than the old cost, but if the address
meets that condition and if propagating at this point in the pipeline is
a win on x86, then wouldn't it be a win for other targets too?
I agree with Richard here.  I can't see a strong reason why this should 
be a target hook.


Perhaps part of the issue here is the address costing metrics may not 
have enough context to make good decisions.  In which case what context 
do they need?


Jeff


Re: [PATCH, PR target/65103, 2/3] Propagate address constants into loops for i386

2015-03-17 Thread Jeff Law

On 03/15/2015 02:30 PM, Richard Sandiford wrote:

Ilya Enkovich enkovich@gmail.com writes:

This patch allows propagation of loop invariants for i386 if propagated
value is a constant to be used in address operand.  Bootstrapped and
tested on x86_64-unknown-linux-gnu.  OK for trunk or stage 1?


Is it necessary for this to be a target hook?  The concept doesn't seem
particularly target-specific.  We should only propagate into the address
if the new cost is no greater than the old cost, but if the address
meets that condition and if propagating at this point in the pipeline is
a win on x86, then wouldn't it be a win for other targets too?

Agreed.  And unless this is a regression, it should be queued for stage1.

jeff


Re: [PATCH, PR target/65103, 2/3] Propagate address constants into loops for i386

2015-03-15 Thread Richard Sandiford
Ilya Enkovich enkovich@gmail.com writes:
 This patch allows propagation of loop invariants for i386 if propagated
 value is a constant to be used in address operand.  Bootstrapped and
 tested on x86_64-unknown-linux-gnu.  OK for trunk or stage 1?

Is it necessary for this to be a target hook?  The concept doesn't seem
particularly target-specific.  We should only propagate into the address
if the new cost is no greater than the old cost, but if the address
meets that condition and if propagating at this point in the pipeline is
a win on x86, then wouldn't it be a win for other targets too?

Thanks,
Richard


Re: [PATCH, PR target/65103, 2/3] Propagate address constants into loops for i386

2015-03-10 Thread Ilya Enkovich
On 10 Mar 18:00, Ilya Enkovich wrote:
 Hi,
 
 This patch allows propagation of loop invariants for i386 if propagated value 
 is a constant to be used in address operand.  Bootstrapped and tested on 
 x86_64-unknown-linux-gnu.  OK for trunk or stage 1?
 
 Thanks,
 Ilya

Updated ChangeLog and test.

Thanks
Ilya
--
gcc/

2015-03-10  Ilya Enkovich  ilya.enkov...@intel.com

PR target/65103
* target.def (try_fwprop_invariant): New.
* doc/tm.texi.in (TARGET_TRY_FWPROP_INVARIANT): New.
* doc/tm.texi: Regenerate.
* fwprop.c (forward_propagate_into): Propagate loop
invariants if a target says so.
* hooks.h (hook_bool_rtx_bool_false): New.
* hooks.c (hook_bool_rtx_bool_false): New.
* config/i386/i386.c (ix86_try_fwprop_invariant): New.
(TARGET_TRY_FWPROP_INVARIANT): New.

gcc/testsuite/

2015-03-10  Ilya Enkovich  ilya.enkov...@intel.com

PR target/65103
* gcc.target/i386/pr65103-2.c: New.


diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 06eacd0..b3971b8 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -12977,6 +12977,16 @@ ix86_address_cost (rtx x, machine_mode, addr_space_t, 
bool)
 
   return cost;
 }
+
+static bool
+ix86_try_fwprop_invariant (rtx def_set, bool address_use)
+{
+  if (address_use  GET_CODE (SET_SRC (def_set)) == CONST)
+return true;
+
+  return false;
+}
+
 
 /* Allow {LABEL | SYMBOL}_REF - SYMBOL_REF-FOR-PICBASE for Mach-O as
this is used for to form addresses to local data when -fPIC is in
@@ -52208,6 +52218,9 @@ ix86_initialize_bounds (tree var, tree lb, tree ub, 
tree *stmts)
 #undef TARGET_ABSOLUTE_BIGGEST_ALIGNMENT
 #define TARGET_ABSOLUTE_BIGGEST_ALIGNMENT 512
 
+#undef TARGET_TRY_FWPROP_INVARIANT
+#define TARGET_TRY_FWPROP_INVARIANT ix86_try_fwprop_invariant
+
 struct gcc_target targetm = TARGET_INITIALIZER;
 
 #include gt-i386.h
diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi
index 6c5bfab..59602c4 100644
--- a/gcc/doc/tm.texi
+++ b/gcc/doc/tm.texi
@@ -6421,6 +6421,12 @@ should probably only be given to addresses with 
different numbers of
 registers on machines with lots of registers.
 @end deftypefn
 
+@deftypefn {Target Hook} bool TARGET_TRY_FWPROP_INVARIANT (rtx @var{def_set}, 
bool @var{address_use})
+This hooks tells if it may worth to propagate loop invariant
+@var{def_set} into the loop.  @var{address_use} is 1 if values is
+used in an address operand.
+@end deftypefn
+
 @node Scheduling
 @section Adjusting the Instruction Scheduler
 
diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in
index 8d6dfbc..56e73df 100644
--- a/gcc/doc/tm.texi.in
+++ b/gcc/doc/tm.texi.in
@@ -4722,6 +4722,8 @@ Define this macro if a non-short-circuit operation 
produced by
 
 @hook TARGET_ADDRESS_COST
 
+@hook TARGET_TRY_FWPROP_INVARIANT
+
 @node Scheduling
 @section Adjusting the Instruction Scheduler
 
diff --git a/gcc/fwprop.c b/gcc/fwprop.c
index fc64ec9..d006200 100644
--- a/gcc/fwprop.c
+++ b/gcc/fwprop.c
@@ -1365,8 +1365,17 @@ forward_propagate_into (df_ref use)
   if (DF_REF_IS_ARTIFICIAL (def))
 return false;
 
+  def_insn = DF_REF_INSN (def);
+  if (multiple_sets (def_insn))
+return false;
+  def_set = single_set (def_insn);
+  if (!def_set)
+return false;
+
   /* Do not propagate loop invariant definitions inside the loop.  */
-  if (DF_REF_BB (def)-loop_father != DF_REF_BB (use)-loop_father)
+  if (DF_REF_BB (def)-loop_father != DF_REF_BB (use)-loop_father
+   !targetm.try_fwprop_invariant (def_set,
+   DF_REF_TYPE (use) != DF_REF_REG_USE))
 return false;
 
   /* Check if the use is still present in the insn!  */
@@ -1379,13 +1388,6 @@ forward_propagate_into (df_ref use)
   if (!reg_mentioned_p (DF_REF_REG (use), parent))
 return false;
 
-  def_insn = DF_REF_INSN (def);
-  if (multiple_sets (def_insn))
-return false;
-  def_set = single_set (def_insn);
-  if (!def_set)
-return false;
-
   /* Only try one kind of propagation.  If two are possible, we'll
  do it on the following iterations.  */
   if (forward_propagate_and_simplify (use, def_insn, def_set)
diff --git a/gcc/hooks.c b/gcc/hooks.c
index 824aeb0..1f32701 100644
--- a/gcc/hooks.c
+++ b/gcc/hooks.c
@@ -302,6 +302,12 @@ hook_bool_tree_tree_false (tree a ATTRIBUTE_UNUSED, tree b 
ATTRIBUTE_UNUSED)
 }
 
 bool
+hook_bool_rtx_bool_false (rtx, bool)
+{
+  return false;
+}
+
+bool
 hook_bool_tree_tree_true (tree a ATTRIBUTE_UNUSED, tree b ATTRIBUTE_UNUSED)
 {
   return true;
diff --git a/gcc/hooks.h b/gcc/hooks.h
index 8c929e8..9c32489 100644
--- a/gcc/hooks.h
+++ b/gcc/hooks.h
@@ -59,6 +59,7 @@ extern bool hook_bool_rtx_insn_int_false (rtx_insn *, int);
 extern bool hook_bool_uintp_uintp_false (unsigned int *, unsigned int *);
 extern bool hook_bool_rtx_int_int_int_intp_bool_false (rtx, int, int, int,
   int *, bool);
+extern bool hook_bool_rtx_bool_false (rtx, bool);
 

[PATCH, PR target/65103, 2/3] Propagate address constants into loops for i386

2015-03-10 Thread Ilya Enkovich
Hi,

This patch allows propagation of loop invariants for i386 if propagated value 
is a constant to be used in address operand.  Bootstrapped and tested on 
x86_64-unknown-linux-gnu.  OK for trunk or stage 1?

Thanks,
Ilya
--
gcc/

2015-03-10  Ilya Enkovich  ilya.enkov...@intel.com

PR target/65103
* gcc/target.def (try_fwprop_invariant): New.
* gcc/doc/tm.texi.in (TARGET_TRY_FWPROP_INVARIANT): New.
* gcc/doc/tm.texi: Regenerate.
* gcc/fwprop.c (forward_propagate_into): Propagate loop
invariants if a target says so.
* gcc/hooks.h (hook_bool_rtx_bool_false): New.
* gcc/hooks.c (hook_bool_rtx_bool_false): New.
* gcc/config/i386/i386.c (ix86_try_fwprop_invariant): New.
(TARGET_TRY_FWPROP_INVARIANT): New.

gcc/testsuite/

2015-03-10  Ilya Enkovich  ilya.enkov...@intel.com

PR target/65103
* gcc.target/i386/pr65103-2.c: New.


diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 06eacd0..b3971b8 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -12977,6 +12977,16 @@ ix86_address_cost (rtx x, machine_mode, addr_space_t, 
bool)
 
   return cost;
 }
+
+static bool
+ix86_try_fwprop_invariant (rtx def_set, bool address_use)
+{
+  if (address_use  GET_CODE (SET_SRC (def_set)) == CONST)
+return true;
+
+  return false;
+}
+
 
 /* Allow {LABEL | SYMBOL}_REF - SYMBOL_REF-FOR-PICBASE for Mach-O as
this is used for to form addresses to local data when -fPIC is in
@@ -52208,6 +52218,9 @@ ix86_initialize_bounds (tree var, tree lb, tree ub, 
tree *stmts)
 #undef TARGET_ABSOLUTE_BIGGEST_ALIGNMENT
 #define TARGET_ABSOLUTE_BIGGEST_ALIGNMENT 512
 
+#undef TARGET_TRY_FWPROP_INVARIANT
+#define TARGET_TRY_FWPROP_INVARIANT ix86_try_fwprop_invariant
+
 struct gcc_target targetm = TARGET_INITIALIZER;
 
 #include gt-i386.h
diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi
index 6c5bfab..59602c4 100644
--- a/gcc/doc/tm.texi
+++ b/gcc/doc/tm.texi
@@ -6421,6 +6421,12 @@ should probably only be given to addresses with 
different numbers of
 registers on machines with lots of registers.
 @end deftypefn
 
+@deftypefn {Target Hook} bool TARGET_TRY_FWPROP_INVARIANT (rtx @var{def_set}, 
bool @var{address_use})
+This hooks tells if it may worth to propagate loop invariant
+@var{def_set} into the loop.  @var{address_use} is 1 if values is
+used in an address operand.
+@end deftypefn
+
 @node Scheduling
 @section Adjusting the Instruction Scheduler
 
diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in
index 8d6dfbc..56e73df 100644
--- a/gcc/doc/tm.texi.in
+++ b/gcc/doc/tm.texi.in
@@ -4722,6 +4722,8 @@ Define this macro if a non-short-circuit operation 
produced by
 
 @hook TARGET_ADDRESS_COST
 
+@hook TARGET_TRY_FWPROP_INVARIANT
+
 @node Scheduling
 @section Adjusting the Instruction Scheduler
 
diff --git a/gcc/fwprop.c b/gcc/fwprop.c
index fc64ec9..d006200 100644
--- a/gcc/fwprop.c
+++ b/gcc/fwprop.c
@@ -1365,8 +1365,17 @@ forward_propagate_into (df_ref use)
   if (DF_REF_IS_ARTIFICIAL (def))
 return false;
 
+  def_insn = DF_REF_INSN (def);
+  if (multiple_sets (def_insn))
+return false;
+  def_set = single_set (def_insn);
+  if (!def_set)
+return false;
+
   /* Do not propagate loop invariant definitions inside the loop.  */
-  if (DF_REF_BB (def)-loop_father != DF_REF_BB (use)-loop_father)
+  if (DF_REF_BB (def)-loop_father != DF_REF_BB (use)-loop_father
+   !targetm.try_fwprop_invariant (def_set,
+   DF_REF_TYPE (use) != DF_REF_REG_USE))
 return false;
 
   /* Check if the use is still present in the insn!  */
@@ -1379,13 +1388,6 @@ forward_propagate_into (df_ref use)
   if (!reg_mentioned_p (DF_REF_REG (use), parent))
 return false;
 
-  def_insn = DF_REF_INSN (def);
-  if (multiple_sets (def_insn))
-return false;
-  def_set = single_set (def_insn);
-  if (!def_set)
-return false;
-
   /* Only try one kind of propagation.  If two are possible, we'll
  do it on the following iterations.  */
   if (forward_propagate_and_simplify (use, def_insn, def_set)
diff --git a/gcc/hooks.c b/gcc/hooks.c
index 824aeb0..1f32701 100644
--- a/gcc/hooks.c
+++ b/gcc/hooks.c
@@ -302,6 +302,12 @@ hook_bool_tree_tree_false (tree a ATTRIBUTE_UNUSED, tree b 
ATTRIBUTE_UNUSED)
 }
 
 bool
+hook_bool_rtx_bool_false (rtx, bool)
+{
+  return false;
+}
+
+bool
 hook_bool_tree_tree_true (tree a ATTRIBUTE_UNUSED, tree b ATTRIBUTE_UNUSED)
 {
   return true;
diff --git a/gcc/hooks.h b/gcc/hooks.h
index 8c929e8..9c32489 100644
--- a/gcc/hooks.h
+++ b/gcc/hooks.h
@@ -59,6 +59,7 @@ extern bool hook_bool_rtx_insn_int_false (rtx_insn *, int);
 extern bool hook_bool_uintp_uintp_false (unsigned int *, unsigned int *);
 extern bool hook_bool_rtx_int_int_int_intp_bool_false (rtx, int, int, int,
   int *, bool);
+extern bool hook_bool_rtx_bool_false (rtx, bool);
 extern bool hook_bool_tree_tree_false (tree, tree);
 extern bool