562152 - Remove typedef LInsp

Status: RESOLVED FIXED

(Core Graveyard :: Nanojit, defect, P4)

Description

[Edwin Smith]

The inconsistent use of this typedef is bothering us, and it also appears to confuse Eclipse CDT indexing when not used consistently between a definition and declaration of a method.  

Time to go!

Comment 1

[Edwin Smith]

in TR, CDT find references says:

CodegenLIR.cpp (56 matches)
Assembler.cpp (15 matches)
Assembler.h (31 matches)
LIR.cpp (140 matches)
LIR.h (151 matches)

Comment 2

[Edwin Smith]

Attachment: Remove uses of LInsp in CodegenLIR.cpp

no big rush on this review, posting now to keep my pipeline full

Comment 3

[Edwin Smith]

Comment on attachment 444530 [details] [diff] [review]
Remove uses of LInsp in CodegenLIR.cpp

TR: http://hg.mozilla.org/tamarin-redux/rev/dd2020e89b86

Comment 4

[Edwin Smith]

Attachment: Remove use of LInsp in nanojit

Note: Upon request, I can this as a series of patches over time, to reduce merge collisions.  Just easier to post the whole thing for review.

noteworthy:
* two places in LIR.h required fixing more elaborate than search & replace:   LInsp a, b;   => LIns *a, *b;
* testing on TR sandbox tonight, will offer a TR patch too.
* typedef LIns* LInsp; is still present in LIR.h until all uses are removed.  TR is clean.

Comment 5

[Nicholas Nethercote [inactive]]

Comment on attachment 450180 [details] [diff] [review]
Remove use of LInsp in nanojit

>diff -r 0a8efeddb4d9 nanojit/LIR.cpp
>--- a/nanojit/LIR.cpp	Wed Jun 09 09:30:23 2010 +0100
>+++ b/nanojit/LIR.cpp	Wed Jun 09 15:02:15 2010 -0400
>@@ -930,10 +930,10 @@
> 
>     // Simplify operator if possible.  Always return NULL if overflow is possible.
> 
>-    LIns* ExprFilter::simplifyOverflowArith(LOpcode op, LInsp *opnd1, LInsp *opnd2)
>+    LIns* ExprFilter::simplifyOverflowArith(LOpcode op, LIns* *opnd1, LIns* *opnd2)

Ew, gross.  "LIns** opnd1" (my preference) or "LIns **opnd1" please.  

>@@ -2021,12 +2021,12 @@
>     {
>         const uint32_t oldcap = m_cap[kind];
>         m_cap[kind] <<= 1;
>-        LInsp *oldlist = m_list[kind];
>-        m_list[kind] = new (alloc) LInsp[m_cap[kind]];
>-        VMPI_memset(m_list[kind], 0, m_cap[kind] * sizeof(LInsp));
>+        LIns* *oldlist = m_list[kind];

Here too!

>-        LIns* simplifyOverflowArith(LOpcode op, LInsp *opnd1, LInsp *opnd2);
>+        LIns* simplifyOverflowArith(LOpcode op, LIns* *opnd1, LIns* *opnd2);

And here.

And can you grep for other occurrences of this, in case I missed any?


The rest looks good.  Furthermore, LInsp isn't used anywhere in TM (I just
tested) so you can remove it immediately.

Comment 6

[Edwin Smith]

(In reply to comment #5)
> Ew, gross.  "LIns** opnd1" (my preference) or "LIns **opnd1" please.  
> And can you grep for other occurrences of this, in case I missed any?

gross indeed, i'll fix the ones you found and grep for more.  hazards of search/replace.

> The rest looks good.  Furthermore, LInsp isn't used anywhere in TM (I just
> tested) so you can remove it immediately.

hokay!

Comment 7

[Edwin Smith]

NJ: http://hg.mozilla.org/projects/nanojit-central/rev/63a0a36e4637

Comment 8

[Nicholas Nethercote [inactive]]

TM: http://hg.mozilla.org/tracemonkey/rev/47e02e48437a

Comment 9

[Edwin Smith]

TR: http://hg.mozilla.org/tamarin-redux/rev/7d2ad7374035

Comment 10

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/47e02e48437a