steven_wu added a comment. In D108881#2988026 <https://reviews.llvm.org/D108881#2988026>, @tejohnson wrote:
> In D108881#2988016 <https://reviews.llvm.org/D108881#2988016>, @steven_wu > wrote: > >> In D108881#2987990 <https://reviews.llvm.org/D108881#2987990>, @mnadeem >> wrote: >> >>> In D108881#2973735 <https://reviews.llvm.org/D108881#2973735>, @steven_wu >>> wrote: >>> >>>> In D108881#2973719 <https://reviews.llvm.org/D108881#2973719>, @mnadeem >>>> wrote: >>>> >>>>> In D108881#2973516 <https://reviews.llvm.org/D108881#2973516>, @steven_wu >>>>> wrote: >>>>> >>>>>> I will do a cleanup of `parseLTOMode` function since we don't need a >>>>>> `OptPos` parameter anymore. There are few minor places references >>>>>> `OPT_flto` or `OPT_foffload_lto` can be cleaned up too. >>>>> >>>>> Will you incorporate the functional changes in this patch? Or is there >>>>> still a need for this change? >>>> >>>> The current change set in this review is functional change while the >>>> cleanup I want is not functional after the rewrite the old option as >>>> Alias. Once flto is the alias, there is no need to handle that in the >>>> driver and those might actually become source of bug in the future. >>>> >>>> I think it would be good to do the cleanup in the same commit unless you >>>> have compelling reason not to. >>> >>> Hi @steven_wu any idea about the timeline? >>> >>> This issue is blocking some internal work, and assuming that it will take >>> longer to get a full fix, I would prefer it if this change could go in on >>> its own. >>> Otherwise I am good with doing everything in the same commit. >> >> I expect the cleanup is very very simple and that is why I am suggested to >> do in the same commit. I am ok with a followup fix as well. > > I think there might be some high level confusion. @steven_wu earlier in the > thread you said "I will do a cleanup", but I think you were asking @mnadeem > to do the cleanup here in this patch. Ah, I see. I probably type the reply from my phone do I didn't double check. I really mean "I would suggest @mnadeem to do a cleanup in the same commit". Sorry about the confusion. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108881/new/ https://reviews.llvm.org/D108881 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
