Hao,
What are you trying to change? What are you trying to enable?
Section Anchors already are enabled in the rs6000 port.
MIN_ANCHOR_OFFSET and MAX_ANCHOR_OFFSET already are defined with other
values and for good reason. From rs6000.c:
/* Use a 32-bit anchor range. This leads to sequences like:
addis tmp,anchor,high
add dest,tmp,low
where tmp itself acts as an anchor, and can be shared between
accesses to the same 64k page. */
#undef TARGET_MIN_ANCHOR_OFFSET
#define TARGET_MIN_ANCHOR_OFFSET -0x7fffffff - 1
#undef TARGET_MAX_ANCHOR_OFFSET
#define TARGET_MAX_ANCHOR_OFFSET 0x7fffffff
targetm.min_anchor_offset
targetm.max_anchor_offset
seem to be defined correctly when I check.
I hope that Richard Sandiford doesn't mind my quoting from our private
email discussion from 2008. He originally used the values that you
are trying to override.
I'd somehow forgetten that PowerPC had addis,
and was thinking it was like MIPS, where the code to add a large offset
looks like:
lui r2,%hi(offset)
addiu r2,r2,r1
followed by uses of r2 + %lo(offset). That's the situation I was referring
to in my mail. This takes two instructions each time you need to compute
"anchor + %hi(offset)", whereas storing "anchor + %hi(offset)" in the
constant pool will take one word for that constant pool entry and one
for each load. Using arithmetic to calculate the anchor would therefore
tend to increase code size for MIPS.
The situation is similar for ARM and MIPS16, where constant pool
accesses are cheap compared to sequences that add large offsets.
I really think it is useful to have the flexibility to create
more than one anchor per block.
I agree that my PowerPC definitions
of the {min,max}_anchor_offset hooks are wrong, and should indeed
be the full range.
My assumption was that if a target needs to split an address
calculation into several instructions, the rtl optimisers should CSE
it where appropriate. For example, if we have "anchor + 0x100002"
and "anchor + 0x100004", and we have a RISC target only supports 16-bit
offsets, that target should already try to calculate the addresses as
"x = anchor + 0x100000; x + 2" and "x = anchor + 0x100000; x + 4".
"x = anchor + 0x10000" can then be optimised as appropriate.
I disagree with your new definitions and I disagree with the manner in
which you are trying to change the values.
Your patch is NOT okay without a lot more explanation and justification.
Thanks, David
On Tue, Mar 16, 2021 at 10:23 PM HAO CHEN GUI <guih...@linux.ibm.com> wrote:
>
> Segher,
>
> The const_anchor should work on both 64 and 32 bit. I think the
> constant loading is cheap on 32 bit platform, so I only enabled it on
> 64 bit. I will add a test case and verify the patch on Darwin and AIX.
> Thanks.
>
> On 17/3/2021 上午 12:35, Segher Boessenkool wrote:
> > Hi!
> >
> > On Mon, Mar 15, 2021 at 11:11:32AM +0800, HAO CHEN GUI via Gcc-patches
> > wrote:
> >> This patch adds const_anchor for rs6000. The const_anchor is used
> >> in cse pass.
> > 1) This isn't suitable for stage 4.
> > 2) Please add a test case, which shows what it does, that it is useful.
> > 3) Does this work on other OSes than Linux? What about Darwin and AIX?
> >
> >> diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
> >> index ec068c58aa5..2b2350c53ae 100644
> >> --- a/gcc/config/rs6000/rs6000.c
> >> +++ b/gcc/config/rs6000/rs6000.c
> >> @@ -4911,6 +4911,13 @@ rs6000_option_override_internal (bool global_init_p)
> >> warning (0, "%qs is deprecated and not recommended in any
> >> circumstances",
> >> "-mno-speculate-indirect-jumps");
> >>
> >> + if (TARGET_64BIT)
> >> + {
> >> + targetm.min_anchor_offset = -32768;
> >> + targetm.max_anchor_offset = 32767;
> >> + targetm.const_anchor = 0x8000;
> >> + }
> > Why only on 64 bit? Why these choices?
> >
> >
> > Segher
