On 2014/02/27 11:49:28, rmcilroy wrote:
https://codereview.chromium.org/164793003/diff/190001/src/a64/macro-assembler-a64.cc
File src/a64/macro-assembler-a64.cc (right):
https://codereview.chromium.org/164793003/diff/190001/src/a64/macro-assembler-a64.cc#newcode4993
src/a64/macro-assembler-a64.cc:4993: FPTmpList()->Combine(kCallerSavedFP);
On 2014/02/27 11:27:16, jbramley wrote:
> On 2014/02/27 11:17:32, rmcilroy wrote:
> > Do we really need all these registers as temps? Seems quite hacky to
modify
> the
> > TmpList manually here and rely on the UseScratchRegisterScope to
restore
it.
>
> No, it doesn't need them, but it's easier to provide them all than to
select
a
> couple from the list. We have to preserve all caller-saved registers
for the
> call anyway, so they'll all potentially be clobbered.
>
> This should have been temps.Include() rather than a TmpList() hack, and
I
would
> have changed it had I not removed Include().
>
> > Maybe you could just use:
> > Register tmp = GetAllocatableRegisterThatIsNotOneOf(arg0..arg3)?
>
> GetAllocatableRegisterThatIsNotOneOf doesn't work because the list of
> allocatable scratch registers includes some callee-saved registers.
>
> > Not sure if any of these other macroassembler calls could use tmp
registers
> > internally as well though?
>
> Theoretically they might, but in practice, I think the only one here
that
uses
> scratch registers is PrintfNoPreserve.
I still don't like this - it feels like a hack and a misuse of
UseScratchRegisterScope. I would prefer if you made it possible to
specify an
allocatable set of registers to GetAllocatableRegisterThatIsNotOneOf, and
then
use that passing in kCallerSaved. If it really isn't possible then I
suppose
I
can live with it if it's only in PrintF, but I would this change.
How many scratch registers do I request? I need one here, to preserve NZCV.
I
think PrintfNoPreserve needs two (indirectly), but I'm not certain of that,
and
it depends on the implementation of the MacroAssembler functions that it
uses.
That's three calls to GetAllocatableRegisterThatIsNotOneOf, and it's
inflexible
in that PrintfNoPreserve might be able to be more efficient with more.
Alternatively, it's wasteful if PrintfNoPreserve only uses one.
Would it be better if I prepared the list first, then passed it in?
{ CPURegList scratch = kCallerSaved;
if (scratch.IncludesAliasOf(arg0)) scratch.Remove(arg0);
if (scratch.IncludesAliasOf(arg1)) scratch.Remove(arg1);
if (scratch.IncludesAliasOf(arg2)) scratch.Remove(arg2);
if (scratch.IncludesAliasOf(arg3)) scratch.Remove(arg3);
TmpList()->set_list(scratch.list());
CPURegList fpscratch = kCallerSavedFP;
if (fpscratch.IncludesAliasOf(arg0)) fpscratch.Remove(arg0);
if (fpscratch.IncludesAliasOf(arg1)) fpscratch.Remove(arg1);
if (fpscratch.IncludesAliasOf(arg2)) fpscratch.Remove(arg2);
if (fpscratch.IncludesAliasOf(arg3)) fpscratch.Remove(arg3);
FPTmpList()->set_list(fpscratch.list());
...
TmpList()->set_list(0);
FPTmpList()->set_list(0);
}
That avoids UseScratchRegisterScope at this level. It's very verbose, but it
might be clearer, and it doesn't mix TmpList() with UseScratchRegisterScope.
https://codereview.chromium.org/164793003/
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/groups/opt_out.
