[PATCH] D99967: [Flang] Changes to mangling code

[Kiran Chandramohan via Phabricator via llvm-commits]

kiranchandramohan added a comment.



>> I am not disagreeing with the two points that you suggest. The partial end-to-end support was suggested but did not get the flang community approval.
>
> Got any links to records of the discussion there? (perhaps this is an awkward case of having flang commits going to the LLVM-commits list, but the flang community conversations aren't happening in llvm-dev or other llvm areas - though good that it catches some community norm differences that we'd like to keep closer to the established LLVM conventions)
>
> It might be worth bringing up this aspect of this kind of incremental development in that conversation, perhaps it changes the weighting of alternatives to some degree.

These discussions mostly happened during the flang technical/biweekly calls that we have.

>> I believe the difficulty included finding small patches which implement end to end functionality that can be upstreamed. For e.g. when i tried to upstream the parse-tree to FIR lowering (https://reviews.llvm.org/D79731) of an empty subroutine in May last year it was a huge patch and there was not much support for it.
>
> Agreed, that seems like a rather large patch - though I wonder if it is as small as it could be (for instance, at a glance - I can't seem to find a use of the AbstractConverter (it's only derived by one type and doesn't seem to be used via references/pointers to the base type - so maybe FirConverter could exist without the extra abstraction as a first pass/minimizing of this patch and the base class could be added in whatever later patch needs that abstraction)
>
> But, yeah - so long as things are tested as they're added, one way or another, it's not deeply problematic - whatever way flang folks would like, just a minor suggestion that it might be worth considering if a more functionally incremental approach would be possible/beneficial.

As I understand there is going to be some discussions going to happen in the next two weeks on trying to establish a better process for upstreaming the Flang code. May be there is enough support code upstream so that we can actually do the functionally incremental approach now. I will raise this during the discussions.

For this particular patch I will try to add some unittests.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D99967/new/

https://reviews.llvm.org/D99967