[llvm] r181753 - This is the first of three patches which creates stubs used for

[Jim Grosbach]

On May 14, 2013, at 4:28 PM, Doug Gilmore <Doug.Gilmore at imgtec.com> 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 at 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 at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130515/32a3beef/attachment.html>