Hi Jonathan, The main issue with your patch is that it can change user expected behaviour, and I can't tell you what is the expected behaviour in Darwin or BSD. If people usually use "unknown" in triples, this will break their builds. If not, this could break the build of someone who does. My advice is to create a "default" mechanism for the affected targets, something that maps "unknwon" to "whatever" (usually "none" but could be "linux" or "win" depending on the rest of the triple). To do that, you'll have to learn what is the expected default on each. You could just completely ignore the expectation and just commit your patch as it is, but only with agreement from the rest of the community (Darwin, x86-Linux and FreeBSD). On ARM+Linux, "unknown" is basically the same as "none", aka "bare-metal". But I can't tell you on anything else. So, while I don't mind the change, people might do. cheers, --renato On 19 June 2014 14:52, Jonathan Roelofs <jonathan at codesourcery.com> wrote:> [+llvm-commits, +cfe-commits] (forgot to add them too) > > > > -------- Original Message -------- > Subject: [cfe-dev] [PATCH] triples for baremetal > Date: Thu, 19 Jun 2014 07:43:44 -0600 > From: Jonathan Roelofs <jonathan at codesourcery.com> > To: Eric Christopher <echristo at gmail.com> > CC: Clang Commits <cfe-commits at cs.uiuc.edu>, Phoebe Buckheister > <llvm at quasiparticle.net>, "cfe-dev at cs.uiuc.edu" <cfe-dev at cs.uiuc.edu>, > "llvmdev at cs.uiuc.edu" <llvmdev at cs.uiuc.edu> > > Eric, > > Attached are patches for llvm and clang that implement this. > > I've made 'none' a component that must be added explicitly (i.e. don't turn > arm-eabi into arm--none-eabi, but rather turn it into arm--unknown-eabi) to > try > to reduce surprises. It also keeps the normalization logic a bit simpler > than it > would otherwise have to be. > > SPIR triples were one place where I was uncertain... I'm not sure if they'd > prefer to use 'none' or rather just omit that part of the triple. So on > those, > I've left them to use Triple::UnknownOS. > > > Cheers, > Jon > > On 6/17/14, 11:11 AM, Eric Christopher wrote: >> >> Agreed. >> >> -eric >> >> On Tue, Jun 17, 2014 at 9:54 AM, Jonathan Roelofs >> <jonathan at codesourcery.com> wrote: >>> >>> [+llvmdev, -llvm-dev] >>> >>> (Oopsies, llvmdev doesn't have a hyphen in it like all the others do) >>> >>> >>> On 6/17/14, 10:45 AM, Jonathan Roelofs wrote: >>>> >>>> >>>> [+llvm-dev, cfe-dev] >>>> >>>> Was "Re: [PATCH] ARM: allow inline atomics on Cortex M" >>>> >>>> On 6/17/14, 10:42 AM, Jonathan Roelofs wrote: >>>>> >>>>> >>>>> >>>>> >>>>> On 6/17/14, 9:35 AM, Renato Golin wrote: >>>>>> >>>>>> >>>>>> On 17 June 2014 16:29, Jonathan Roelofs <jonathan at codesourcery.com> >>>>>> wrote: >>>>>>> >>>>>>> >>>>>>> Attached is what I now think the patch ought to be. >>>>>> >>>>>> >>>>>> >>>>>> Does unknownOS *always* mean bare-metal? >>>>> >>>>> >>>>> I'm not sure. It might be a good time to fork this thread, and start >>>>> another >>>>> about triples for bare-metal... >>>> >>>> >>>> >>>> Personally, I think we ought to add a 'None' entry to the OSType enum >>>> specifically for baremetal, and then map triples like arm-none-eabi and >>>> arm--eabi to it (but not arm-foobar-eabi, for example). >>>> >>>> Thoughts? >>>> >>>> Jon >>>> >>>>>> >>>>>> --renato >>>>>> >>>>> >>>> >>> >>> -- >>> Jon Roelofs >>> jonathan at codesourcery.com >>> CodeSourcery / Mentor Embedded >>> _______________________________________________ >>> LLVM Developers mailing list >>> LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu >>> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev > > > -- > Jon Roelofs > jonathan at codesourcery.com > CodeSourcery / Mentor Embedded > > > -- > Jon Roelofs > jonathan at codesourcery.com > CodeSourcery / Mentor Embedded > > > > _______________________________________________ > LLVM Developers mailing list > LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev >
On 23 Jun 2014, at 15:13, Renato Golin <renato.golin at linaro.org> wrote:> The main issue with your patch is that it can change user expected > behaviour, and I can't tell you what is the expected behaviour in > Darwin or BSD. If people usually use "unknown" in triples, this will > break their builds. If not, this could break the build of someone who > does.I think (Hat: FreeBSD) we only expect to see unknown in the vendor field (e.g. i386-unknown-freebsd). If the OS field is unknown, rather than freebsd, then it's not one of ours and we aren't likely to care. I don't really like the way that we conflate OS with ABI, but then I don't really like anything about triples... David
Joerg Sonnenberger
2014-Jun-23 15:27 UTC
[LLVMdev] [cfe-dev] [PATCH] triples for baremetal
On Mon, Jun 23, 2014 at 03:31:55PM +0100, David Chisnall wrote:> On 23 Jun 2014, at 15:13, Renato Golin <renato.golin at linaro.org> wrote: > > > The main issue with your patch is that it can change user expected > > behaviour, and I can't tell you what is the expected behaviour in > > Darwin or BSD. If people usually use "unknown" in triples, this will > > break their builds. If not, this could break the build of someone who > > does. > > I think (Hat: FreeBSD) we only expect to see unknown in the vendor > field (e.g. i386-unknown-freebsd). If the OS field is unknown, rather > than freebsd, then it's not one of ours and we aren't likely to care. > I don't really like the way that we conflate OS with ABI, but then > I don't really like anything about triples...Except that last rant, I agree with s/FreeBSD/NetBSD. Joerg
On 6/23/14, 8:31 AM, David Chisnall wrote:> On 23 Jun 2014, at 15:13, Renato Golin <renato.golin at linaro.org> wrote: > >> The main issue with your patch is that it can change user expected >> behaviour, and I can't tell you what is the expected behaviour in >> Darwin or BSD. If people usually use "unknown" in triples, this will >> break their builds. If not, this could break the build of someone who >> does.Renato, I would like to go this route, providing I can get support from the community that this is the direction we'd all like to take... I'd rather not make triples more complicated by introducing lots of special cases. Do you know who the right folks are to ask about the SPIR triples? Tim, This patch changes behavior a little for macho_embedded targets (i.e. from OSType::Unknown to OSType::NoneOS). I see that there is existing code to transform triples from things like thumbv7-apple-darwin into thumbv7m-apple-unknown-macho. Do you have an expectation to support users who are using the latter form of triples who would be surprised by the change from thumbv7m-apple-unknown-macho to thumbv7m-apple-none-macho (i.e. their thumbv7m-apple-unknown-macho builds no longer work)? Is this a case where all three triples are to mean the same thing?> > I think (Hat: FreeBSD) we only expect to see unknown in the vendor field (e.g. i386-unknown-freebsd). If the OS field is unknown, rather than freebsd, then it's not one of ours and we aren't likely to care. I don't really like the way that we conflate OS with ABI, but then I don't really like anything about triples...David, thanks! Cheers, Jon> > David >-- Jon Roelofs jonathan at codesourcery.com CodeSourcery / Mentor Embedded
On 23 June 2014 15:31, David Chisnall <David.Chisnall at cl.cam.ac.uk> wrote:> I don't really like the way that we conflate OS with ABI, but then I don't really like anything about triples...This was broken before... "we" didn't do anything... :) I don't know the whole story but it was supposed to be a triple and people started using the third field to mean different things, then they added an interchangeable fourth field, and then it's impossible to distinguish. I cry every time I have to think about it. --renato