Hi Olivier,
On 12/10/18 19:43, Olivier Hainque wrote:
Hi Kyrill,
Thanks for your feedback!
On 12 Oct 2018, at 05:50, Kyrill Tkachov <kyrylo.tkac...@foss.arm.com> wrote:
CC'ing the aarch64 maintainers as they'll have to approve it.
I'm guessing you've tested this in the usual way (bootstrap and test)?
Sorry, I failed to mention the testing indeed. We don't
have a native box at hand, so I'm not really able to
conduct regular bootstrap and dg testing per se.
We have an active gcc-7 aarch64-vxworks port, still, and have
had good ACATS and internal testsuite results for this port
with this patch, so it doesn't break the build and has
the intended effect on a configuration where FIXED_R18 is
redefined to 1.
We also have good build & tests results on aarch64-elf ports,
which don't redefine the macro.
I checked that we have uses of x18 in several places in libgnat.a
on aarch64-elf, both as a static chain and as a scratch, and
the only references to x18 in the VxWorks libgnat.a are from
functions referring to "environ", precisely the platform specific
use which prevents us from using the register otherwise.
I also sanity checked that "make all-gcc" passes (self tests
included) with mainline + the patch for --target=aarch64-elf
--enable-languages=c.
Any reason not to use the TARGET_CONDITIONAL_REGISTER_USAGE interface to adjust
the fixed_regs table instead?
I'm not objecting to your approach here, just interested if you considered and
rejected it.
Hmm, I haven't consciously rejected it. I just went for what
seemed natural at first sight from the elements in place.
In particular, the FIXED_REGS definition comes with a comment
on the use of r18 and it seemed logical to adjust both the comment
and the table there.
I'm happy to move that part to aarch64_conditional_register_usage
if that's considered more canonical of course.
I don't think it's more canonical, and it is a run-time thing, whereas your
patch changes things
at configure time, so there's no runtime overhead.
It seems like I might need to set call_used_registers to 1 as well.
STATIC_CHAIN_REGNUM still needs to be adjusted directly I think.
I think so too, so you'd still need to have these configure-time changes.
If we could make it all runtime that would be clean, but perhaps it's not worth
splicing the two approaches.
I wondered if we could set it to R11 unconditionally and picked
the way ensuring no change for !vxworks ports, especially since I
don't have means to test more than what I described above.
We're working on a transition to gcc-8 and I can certainly port this
and the rest rapidly to verify that we get similar results on the
more recent code base.
So, do you still want to make this change in current trunk? Or will you make
the necessary changes
when contributing the vxworks port?
Thanks,
Kyrill
I'll be more than happy to incorporate suggestions of changes in
that process.
Thanks in advance,
With Kind Regards,
Olivier
