Reed Kotler
2013-May-15 18:36 UTC
[LLVMdev] [llvm] r181753 - This is the first of three patches which creates stubs used for
Hi Jim, There is no "target independent" code in these patches. It's all Mips specific. There is something I'm proposing for AP/NO_APP wrappers but that will just make the stubs look nicer (without the clutter of those wrappers). That is purely a cosmetic issue of the .s files being produced. The discussion has moved back to the main thread so I'll continue there. Thanks for taking to make a detailed answer. Reed On 05/15/2013 10:30 AM, Jim Grosbach wrote:> > On May 14, 2013, at 4:28 PM, Doug Gilmore > <Doug.Gilmore-1AXoQHu6uovQT0dZR+AlfA at public.gmane.org > <mailto:Doug.Gilmore-1AXoQHu6uovQT0dZR+AlfA at public.gmane.org>> wrote: > >>> Hi >>> >>> Hi Reed, >>> >>> I’m confused. There have been multiple very strong objections to >>> having the compiler generate inline asm nodes like this. Did I miss >>> the discussion where that got resolved? >>> >>> -Jim >> Hi Jim, >> >> I went along with this approach of using inline asm generation since >> it allowed us to generate optimal code for the stub functions. >> >> The functions being called contain very little code so it is important >> to generate these stub functions as efficiently as possible. >> >> Having this functionality working sooner rather than later speeds up >> testing since tracking down regressions using only soft-float can be >> very time consuming. We would really like to attend to the test-suite >> regressions so that we start getting clean test-suite runs from our >> build-bots for MIPS16. >> >> Do you think we really need to pull and rework the code? Or can we >> put a FIXME comment in the code and attend to it after we fix all of >> the MIPS16 test-suite regressions? >> > > Hi Doug, > > Thanks for the elaboration on the background. Very helpful for those > (like me) coming a bit late to the discussion. > > The technical discussion looks to be re-booted and off and going, so > I’ll save comments on that aspect for there to avoid forking the > conversation too horribly badly. > > My question is mainly a process one, anyway. The last I saw the > discussion (mainly in the APP/NOAPP thread), multiple folks had > expressed significant reservations about the approach. I hadn’t seen > that discussion come to any conclusions other than Reed expressing > disagreement, so I was confused when this patch landed, so I wanted to > check whether I’d just missed part of the conversation. It’s generally > expected that those sorts of concerns be resolved before a commit > happens. Otherwise it gets harder and harder to change early decisions, > even if it’s towards a better design, due to increased layers of code > depending on the initial patch(es). This is central to how the LLVM > community works, which is why you see a fairly strong reaction when it’s > short circuited. > > Now, there’s a very large amount of leeway for code that’s purely in the > MIPS backend, of course. You guys are the ones maintaining it, so your > preferences naturally carry more weight than those of us in the peanut > gallery. If this series of patches were only ever going to touch > target-specific code, I doubt you’d be seeing anything more than a bit > of mild distaste and suggestions of alternatives. It’s been expressed, > however, that there are target-independent changes coming related to > this patch. That gets those of us responsible for those target > independent layers more directly involved and concerned. That’s the > technical aspect of the discussion that needs resolved before this set > of patches moves too far forward. > > For this specific patch, I’m personally fine with a “FIXME” at the top. > This patch is in the MIPS backend, so as long as you guys are happy with > it, that’s what really matters. I personally think there are superior > solutions, having dealt with many very similar problems in the ARM > backend for Thumb/ARM interworking, but I’m not the one maintaining this > code, so that doesn’t matter quite as much. I would request, however, > that any follow-up patches that touch target-independent code be held > for the time being until things come to a firmer resolution. > > Thanks, > Jim > >>> >>> On May 13, 2013, at 7:00 PM, Reed Kotler <rkotler atmips.com >>> <http://mips.com/>> wrote: >>> >>>> Author: rkotler >>>> Date: Mon May 13 21:00:24 2013 >>>> New Revision: 181753 >>>> >>>> URL:http://llvm.org/viewvc/llvm-project?rev=181753&view=rev >>>> Log: >>>> This is the first of three patches which creates stubs used for >>>> Mips16/32 floating point interoperability. >> >> >> _______________________________________________ >> llvm-commits mailing list >> llvm-commits-Tmj1lob9twqVc3sceRu5cw at public.gmane.org >> <mailto:llvm-commits-Tmj1lob9twqVc3sceRu5cw at public.gmane.org> >> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits > > > > _______________________________________________ > llvm-commits mailing list > llvm-commits-Tmj1lob9twqVc3sceRu5cw at public.gmane.org > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits >