On Wed, 2012-01-11 at 08:09 +1100, Richard Henderson wrote:
> On 01/11/2012 12:43 AM, Torvald Riegel wrote:
> >> One could steal code from bohem-gc for this.
> >> See GC_get_stack_base in os_dep.c.
> >
> > Thanks for the pointer. I looked at this code, and it seems fairly
> > complex given the dependencies on OS/libc and OS/libc behavior. From a
> > maintenance point-of-view, does it make sense to copy that complexity
> > into libitm? boehm-gc is used in GCC, so perhaps that's not much of a
> > problem, however. I also looked at glibc's memcpy implementations, and
> > copying those plus a simple byte-wise copy for the generic case could be
> > also a fairly clean solution.
> > Also, is the license compatible with the GPL wrt. mixing sources?
>
> From the maintenance point of view, I do think it makes sense to copy.
> As for the license, I expect we'd want to copy into a separate file so
> that we can keep things vaguely separated.
>
> > What about keeping the patch/hack that I posted for now, creating a PR,
> > and looking at this again for another release?
>
> I suppose that's not unreasonable. Ok with...
>
> > +static inline void *
> > +mask_stack_bottom(gtm_thread *tx)
> > +{
> > + return (uint8_t*)__builtin_dwarf_cfa() - 128;
> > +}
>
> Not only can this not be inline, it must be out-of-line. Otherwise you're
> not including the stack frame of gtm_undolog::rollback much less memcpy. You
> could get this result inline if you specialized for the arch by looking at
> the hard stack pointer register, but __builtin_dwarf_cfa is at the wrong end
> of the stack.
Oops. Based on the previous code I thought it would return the bottom
end of the stack frame. Made this function noinline and moved it to
config/generic/tls.c.
>
> You might as well make the fudge factor a lot larger. Like 4-8k.
Opted for 256 because too large might prevents undos to more than
expected with tight stack space.
>
> > + if (likely(ptr > top || (uint8_t*)ptr + len <=bot))
>
> Missing space before "bot".
Committed with those changes as rev 183172. Created PR libitm/51855.
