Daniel Sanders via llvm-dev
2016-Jun-15 10:24 UTC
[llvm-dev] Sincos for X86_64's GNUX32 and ARM's GNUEABI/GNUEABIHF enviroments
Hi, While writing http://reviews.llvm.org/D20916, I stumbled across some code affecting ARM and X86_64 environments that looks like it might be unintentional. I thought I should ask about it here since that patch has a '[mips]' tag and therefore might not be noticed by someone who knows. I've noticed that the GNUX32 and GNUEABI/GNUEABIHF environments don't make use of the sincos libcall like the GNU environment does because the guarding condition for this is 'TT.getEnvironment() == Triple::GNU'. The comment in canCombineSinCosLibcall() in LegalizeDAG.cpp suggests that it's intending to use sincos for all GNU environments so I'm wondering whether the exclusion of GNUX32/GNUEABI/GNUEABIHF is deliberate or not. Can anyone confirm? Thanks.
Renato Golin via llvm-dev
2016-Jun-16 08:43 UTC
[llvm-dev] Sincos for X86_64's GNUX32 and ARM's GNUEABI/GNUEABIHF enviroments
On 15 June 2016 at 11:24, Daniel Sanders via llvm-dev <llvm-dev at lists.llvm.org> wrote:> I've noticed that the GNUX32 and GNUEABI/GNUEABIHF environments don't make use of the sincos libcall like the GNU environment does because the guarding condition for this is 'TT.getEnvironment() == Triple::GNU'. The comment in canCombineSinCosLibcall() in LegalizeDAG.cpp suggests that it's intending to use sincos for all GNU environments so I'm wondering whether the exclusion of GNUX32/GNUEABI/GNUEABIHF is deliberate or not.Hi Daniel, I don't remember anything deliberate about that. But I haven't been playing around sin/cos much. To me, it looks like an omission more than anything. cheers, --renato
Daniel Sanders via llvm-dev
2016-Jun-16 13:58 UTC
[llvm-dev] Sincos for X86_64's GNUX32 and ARM's GNUEABI/GNUEABIHF enviroments
> -----Original Message----- > From: Renato Golin [mailto:renato.golin at linaro.org] > Sent: 16 June 2016 09:44 > To: Daniel Sanders > Cc: llvm-dev at lists.llvm.org > Subject: Re: [llvm-dev] Sincos for X86_64's GNUX32 and ARM's > GNUEABI/GNUEABIHF enviroments > > On 15 June 2016 at 11:24, Daniel Sanders via llvm-dev > <llvm-dev at lists.llvm.org> wrote: > > I've noticed that the GNUX32 and GNUEABI/GNUEABIHF environments > don't make use of the sincos libcall like the GNU environment does because > the guarding condition for this is 'TT.getEnvironment() == Triple::GNU'. The > comment in canCombineSinCosLibcall() in LegalizeDAG.cpp suggests that it's > intending to use sincos for all GNU environments so I'm wondering whether > the exclusion of GNUX32/GNUEABI/GNUEABIHF is deliberate or not. > > Hi Daniel, > > I don't remember anything deliberate about that. But I haven't been > playing around sin/cos much. To me, it looks like an omission more > than anything. > > cheers, > --renatoThanks. Seeing as you think it's an omission as well, I've posted a patch at http://reviews.llvm.org/D21431. I've also realised I had the meaning of canCombineSinCosLibcall() backwards. GNUX32/GNUEABI/GNUEABIHF are combining sin+cos into sincos in a situation where GNU considers it to be unsafe.
Possibly Parallel Threads
- [LLVMdev] [RFC PATCH] X32 ABI support for Clang/compiler-rt (Clang patch)
- [LLVMdev] Upstreaming x32 ABI support
- [LLVMdev] [RFC PATCH] X32 ABI support for Clang/compiler-rt
- [LLVMdev] X32 ABI support for Clang/compiler-rt (re: clang patch)
- [LLVMdev] sincos functions