On Wed, 4 Aug 2021 15:56:49 +0200
Thomas Schwinge <tho...@codesourcery.com> wrote:

> > This version of the patch [...]
> > avoids moving SESE-region finding code out
> > of the NVPTX backend  
> 
> So that's 'struct bb_sese' and following functions.
> 
> > since that code isn't used by the middle-end worker
> > partitioning neutering/broadcasting implementation yet.  
> 
> I understand correctly that "isn't used [...] yet" means that (a)
> "this isn't implemented yet" (on og11 etc.), and doesn't mean (b) "is
> missing from this patch submission"?  ... thus from (a) it follows
> that we may later also drop from the og11 branch these changes?

Yes, the former -- the SESE region-finding code isn't used anywhere for
the middle-end worker partitioning support. Thus if we happen to have
two adjacent blocks that both use worker-single mode, we will
conditionalise them to run on one worker only separately, with
redundant tests/branches.

I'm not sure how often that happens in practice. We don't need to
handle vector-single mode for GCN, which possibly means that the
SESE-finding code's ability to skip entire inner loop nests (IIRC!) is
unnecessary.

So yes, that code could probably be dropped for og11 too, though
perhaps we should try to evaluate if it would still be useful first.

> Relatedly, a nontrivial amount of data structures/logic/code did get
> duplicated from the nvptx back end, and modified slightly or
> not-so-slightly (RTL vs. GIMPLE plus certain implementation
> "details").
> 
> We should at least cross reference the two instances, to make sure
> that any changes to one are also propagated to the other.  (I'll take
> care.)

OK, thanks,

> And then, do you (or anyone else, of course) happen to have any clever
> idea about how to avoid the duplication, and somehow combine the RTL
> vs. GIMPLE implementations?  Given that we nowadays may use C++ -- do
> you foresee it feasible to have an abstract base class capturing
> basically the data structures, logic, common code, and then
> RTL-specialized plus GIMPLE-specialized classes inheriting from that?

I suppose one could either use "old-style" inheritance, or maybe do
it with templates. There's probably both costs & benefits when it comes
to maintenance, either way -- having this code shared would mean any
changes need testing for both nvptx & GCN targets, and risks making it
harder to follow. OTOH, like you say, changes would only need to be
made in one place.

TBH, I'd spend effort on trying to integrate the SESE code (if it'd be
beneficial) first, before trying to de-duplicate those other bits.

Thanks,

Julian

Reply via email to