On Mon, Apr 12, 2021 at 05:02:38PM -0500, will schmidt wrote:
> On Fri, 2021-04-09 at 17:09 -0400, Michael Meissner wrote:
> > -   li      r8,trampoline_size      /* verify that the trampoline is big 
> > enough */
> > -   cmpw    cr1,r8,r4
> > +   cmpwi   cr1,r4,trampoline_size  /* verify that the trampoline is big 
> > enough */
> 
> Hmm, I spent several minutes trying to determine how cmpw behaves
> differently than cmpwi before noticing you also swapped the
> order of the r4,r8 operands. 
> 
> That seems OK.
> 
> A statement in the description indicating that you used a cmpwi instead
> of a cmpw since you were in the neighborhood would help call that out. 

In general, don't do two unrelated things, esp. when not pointing it out
explicitly.

> The #elif  _CALL_ELF == 2  portion of tramp.S (line 159 or so) has a
> similar compare stanza with respect to the order of operands on the
> compare.  Will this also have a backwards greater-than less-than issue?
> 
>       li      r8,trampoline_size      /* verify that the trampoline is big 
> enough */
>       cmpw    cr1,r8,r4
>       srwi    r4,r4,3         /* # doublewords to move */
>       addi    r9,r3,-8        /* adjust pointer for stdu */
>       mtctr   r4
>       blt     cr1,.Labort

Not sure...  It should use cmpwi as well though, and then it is easier
to see.


Segher

Reply via email to