[Webkit-unassigned] [Bug 198604] Refactoring of architectural Register Information

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=198604

--- Comment #25 from Paulo Matos <pmatos at igalia.com> ---
(In reply to Caio Lima from comment #19)
> Comment on attachment 371733 [details]
> Revised Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=371733&action=review
> 
> LGTM. I left more comments and we also need to figure out why wincario is
> not building.
> 
> >> Source/JavaScriptCore/assembler/ARM64Assembler.h:173
> >> +#define REGISTER_ID(id, nsid, name, r, cs) id,
> > 
> > nit: Since we are not using `r` and `cs`, I think we could remove them.
> 
> Never mind this comment. We need to have those.
> 
> > Source/JavaScriptCore/assembler/MIPSAssembler.h:108
> > +#undef REGISTER_NAME
> 
> Nice!
> 
> > Source/JavaScriptCore/assembler/MIPSRegisters.h:108
> > +    macro(m, nn(fir),   0)                         \
> 
> I think it would be very good have an commentary documenting each param
> here, so we don't need to go to files where this `FOR_EACH` is used to
> figure out what each value means.
> 
> > Source/JavaScriptCore/jit/RegisterSet.cpp:51
> > +#define SET_IF_RESERVED(id, nsid, name, r, cs) if (r) result.set(nsid);
> 
> I think we should rename `r` to `isReserved`.
> 
> > Source/JavaScriptCore/jit/RegisterSet.cpp:113
> > +#define SET_IF_CALLEESAVED(id, nsid, name, r, cs)  if (cs) result.set(nsid);
> 
> I think it would be better rename `cs` to `isCalleeSaved`.


Should have read this comments before commenting to your initial review set. In any case, thanks for the comments. I am preparing a new patch.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20190612/c173052b/attachment.html>