Re: [trans-mem] Beginning of refactoring

[Richard Henderson <rth at redhat dot com>]

On 06/30/2011 04:15 AM, Torvald Riegel wrote:
> On Wed, 2011-06-29 at 16:01 -0700, Richard Henderson wrote:
>> On 05/25/2011 02:10 PM, Torvald Riegel wrote:
>>> Patch 2: _ITM_dropReferences is not sufficiently defined in the ABI. It
>>> seems to target some form of open nesting for txnal wrappers, but the
>>> prose in the ABI specification is unclear. Thus, disable this for now
>>> (aka fatal runtime error), and expect the related tests to fail. Pick it
>>> up again once that the ABI has been improved and the use cases are
>>> clear.
>>
>> Sure, but please actually delete the code rather than just comment it out.
> 
> Fixed in updated patch2.

This patch is ok.

> How it works is that in a TM-method-specific dispatch class we would add
>   CREATE_DISPATCH_METHODS(static, _static)
> in addition to the 
>   CREATE_DISPATCH_METHODS(virtual, )
> that is currently there. In barrier.cc we can then do
>   CREATE_DISPATCH_FUNCTIONS(GTM::serial_dispatch::_, _static)
> and thus bypass the virtual functions, but still keep one implementation
> of the TM method (in the static template functions). This example would
> create functions that just use the serial-mode accesses.

Ah yes, I remember discussing this.

> The trailing underscore at the end of the first param is to make the
> preprocessor happen. Is there a better way to let it accept
> "GTM::abi_disp()->" as argument?

It depends on what else you're passing to the other methods above.

It does seem odd, though, that you're passing some bit of global
state to the virtual dispatch methods (the result of abi_disp())
whereas you're arranging to pass nothing but a name concatenation
to the others.

I'm not sure what to make of your first two examples above, but
let's suppose that the third example is the real one.  In which
case you don't actually need concatenation.  Remove the ## from
the macro and then you can pass in

  GTM::abi_disp()->
and
  GTM::serial_dispatch::

which require no more than adjacency.

>> Why?  We're already in a header that's clearly marked "internal".
> 
> I moved these macros to a common header so libitm_i.h doesn't have to be
> included everywhere. There was some circular dependency after the
> changes (I think in via the barrier definitions for SSE etc.). What's
> the issue that you see with this change?

Not really an issue, it just seemed odd.

> Are the attached updated patches okay for the branch?

Yeah, we can iterate on the points above without re-sending
the entire patch3 again.


r~