David & Segher,
Thanks so much for your explanation. My patch wants to enables the
constant anchor on rs6000 as TARGET_ANCHOR_CONST or targetm.anchor_const
is undefined. I realized that we have addi and addis instructions. So
the range of the offset could be a 32 bit constant.
I put a test case at
https://github.ibm.com/wschmidt/power-gcc/issues/1042#issuecomment-28922825.
It shows how anchor_const can improve asm output. With anchor_const, the
second complex constant loading can be eliminated by cse if it is within
the range of the first one.
Thanks again and looking forward to your advice.
On 18/3/2021 上午 8:57, David Edelsohn wrote:
On Wed, Mar 17, 2021 at 8:26 PM Segher Boessenkool
<seg...@kernel.crashing.org> wrote:
Hi!
On Wed, Mar 17, 2021 at 03:35:30PM -0400, David Edelsohn wrote:
I disagree with your new definitions and I disagree with the manner in
which you are trying to change the values.
Yes.
Your patch is NOT okay without a lot more explanation and justification.
Which is why I said:
1) This isn't suitable for stage 4.
You give a lot more reasons to not want it, but that was enough for me.
2) Please add a test case, which shows what it does, that it is useful.
I meant there is no way we can accept this patch if we aren't shown what
it does, and that that is a good thing.
3) Does this work on other OSes than Linux? What about Darwin and AIX?
And here I meant that there is no way we can accept patches that
influence code generation on all platforms when we have no idea what it
does on most platforms. I did not intend to suggest the patch would be
more acceptable if it was tested on other platforms; I wanted to say it
is not acceptable if it is not.
The main issue is 2). We need to understand what problem this patch is
trying to solve. I'm sure Hao Chen had a reason for doing this patch,
so I'd like to know what it is trying to achieve, what it is trying to
improve!
Investigating this with Segher, I believe that there is some confusion
about the "ANCHOR" macros.
TARGET_MIN_ANCHOR_OFFSET and TARGET_MAX_ANCHOR_OFFSET are not related
to TARGET_ANCHOR_CONST.
Also, TARGET_ANCHOR_CONST can be defined as a macro to trigger the
hook, and doesn't need targetm.anchor_const.
Any change to TARGET_ANCHOR_CONST requires extensive performance
testing. Yes, it presumably fixes the testcase, but the impact on
overall performance is the critical question.
Thanks, David
