<p dir="ltr">Unfortunately even TLI.setUnavailable doesn't work for fabs, only for copysign. I would have thought this would have stopped the conversion to ISD based on the hasOptimizedCodeGen function. </p>
<div class="gmail_quote">On Jun 14, 2016 7:20 PM, "Anton Korobeynikov" <<a href="mailto:anton@korobeynikov.info">anton@korobeynikov.info</a>> wrote:<br type="attribution"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Mostly because there are fabs / fcopysign instructions in many ISAs.<br>
But nothing like this for floor / trunc.<br>
<br>
On Tue, Jun 14, 2016 at 8:15 PM, Ryan Taylor via llvm-dev<br>
<<a href="mailto:llvm-dev@lists.llvm.org">llvm-dev@lists.llvm.org</a>> wrote:<br>
> If I do<br>
><br>
> T.getArch() == xxx<br>
> TLI.setUnavailable(LibFunc::copysign)<br>
><br>
> then this works at generating a call instead of not being able to select the<br>
> ISD::FCOPYSIGN, but I don't know why I don't need to do this for other<br>
> LibFunc functions (such as floor, etc... these generate call just fine)?<br>
><br>
> Thanks,<br>
> Ryan<br>
><br>
><br>
><br>
> On Tue, Jun 14, 2016 at 11:58 AM, Ryan Taylor <<a href="mailto:ryta1203@gmail.com">ryta1203@gmail.com</a>> wrote:<br>
>><br>
>> I'm still not sure why copysign and fabs have to be lowered to a call when<br>
>> they are represented as a call in the IR?<br>
>><br>
>> Looks like the DAG makes them into SDNodes.<br>
>><br>
>><br>
>> On Thu, Jun 9, 2016 at 2:35 PM, Ahmed Bougacha <<a href="mailto:ahmed.bougacha@gmail.com">ahmed.bougacha@gmail.com</a>><br>
>> wrote:<br>
>>><br>
>>> On Thu, Jun 9, 2016 at 11:28 AM, Ryan Taylor <<a href="mailto:ryta1203@gmail.com">ryta1203@gmail.com</a>> wrote:<br>
>>> > I'm assuming that "__builtin_" is a keyword in LLVM. If I have code<br>
>>> > like:<br>
>>> ><br>
>>> > #define acos __builtin_acos<br>
>>> ><br>
>>> > extern double acos(double, double);<br>
>>> ><br>
>>> > double a;<br>
>>> > void foo(float b) {<br>
>>> > a = acos(b);<br>
>>> > }<br>
>>> ><br>
>>> > I never see a call to "__builtin_acos", is LLVM removing the prefix<br>
>>> > __builtin_ ?<br>
>>><br>
>>> Oh, that's a clang thing, not LLVM: yes, I think clang has special<br>
>>> handling for __builtin_*, and for libm functions, just turns them into<br>
>>> a regular function call to the function. That happens in clang<br>
>>> CGBuiltin.cpp, getBuiltinLibFunction.<br>
>>><br>
>>> By the time LLVM sees the call, yes, it's just a call to "@acos".<br>
>>> There's no such thing as "__builtin_" from LLVM's standpoint, it's<br>
>>> purely a C thing.<br>
>>><br>
>>> -Ahmed<br>
>>><br>
>>> > Thanks,<br>
>>> ><br>
>>> > Ryan<br>
>>> ><br>
>>> > On Thu, Jun 9, 2016 at 2:10 PM, Ahmed Bougacha<br>
>>> > <<a href="mailto:ahmed.bougacha@gmail.com">ahmed.bougacha@gmail.com</a>><br>
>>> > wrote:<br>
>>> >><br>
>>> >> On Wed, Jun 8, 2016 at 10:43 AM, Ryan Taylor <<a href="mailto:ryta1203@gmail.com">ryta1203@gmail.com</a>><br>
>>> >> wrote:<br>
>>> >> > Correct, it does check based on OS and triple, what I meant was that<br>
>>> >> > it<br>
>>> >> > might be better to have this info in the target specific files and<br>
>>> >> > have<br>
>>> >> > the<br>
>>> >> > LibraryInfo do a look up of that (like most other sections of the<br>
>>> >> > core<br>
>>> >> > code<br>
>>> >> > do, ie have the tablegen or ISelLowering specify the libs etc..)<br>
>>> >><br>
>>> >> I agree it's not the best place, but one difference is that<br>
>>> >> TargetLibraryInfo is much more about OSes than architectures.<br>
>>> >><br>
>>> >> > I'm not sure I follow about the RTLIB, I'm able to use an intrinsic<br>
>>> >> > for<br>
>>> >> > floor (def int_floor::Intrinsic in IntrinsicsXXX.td) and still use<br>
>>> >> > RTLIB<br>
>>> >> > to<br>
>>> >> > generate the appropriate name for the function (ie __xxx_floor). It<br>
>>> >> > sounds<br>
>>> >> > like you're implying either/or, not both?<br>
>>> >><br>
>>> >> No, I'm just saying that RTLIB only solves the codegen problem; you'll<br>
>>> >> need something else (like your intrinsic?) to have better IR<br>
>>> >> optimizations.<br>
>>> >><br>
>>> >> > I agree, it doesn't seem supported. It looks like I might just need<br>
>>> >> > to<br>
>>> >> > change 'TLI.has' and 'TLI.getName' in order to make this happen<br>
>>> >> > (potentially<br>
>>> >> > removing the prefix here). This goes back to my first point, the TLI<br>
>>> >> > should<br>
>>> >> > be changed to simply get this info generically from the target<br>
>>> >> > information,<br>
>>> >> > you seem to agree with that.<br>
>>> >><br>
>>> >> Hmm, what are you really trying to do? If you want LLVM to recognize<br>
>>> >> your __xxx functions: yes, the cleanest solution is probably to teach<br>
>>> >> TLI and its users to recognize the "custom" names, and mark the<br>
>>> >> functions as available with your custom __xxx names.<br>
>>> >><br>
>>> >> HTH,<br>
>>> >> -Ahmed<br>
>>> >><br>
>>> >> > Thanks,<br>
>>> >> ><br>
>>> >> > Ryan<br>
>>> >> ><br>
>>> >> > On Tue, Jun 7, 2016 at 7:06 PM, Ahmed Bougacha<br>
>>> >> > <<a href="mailto:ahmed.bougacha@gmail.com">ahmed.bougacha@gmail.com</a>><br>
>>> >> > wrote:<br>
>>> >> >><br>
>>> >> >> On Tue, Jun 7, 2016 at 1:57 PM, Ryan Taylor <<a href="mailto:ryta1203@gmail.com">ryta1203@gmail.com</a>><br>
>>> >> >> wrote:<br>
>>> >> >> > Tim,<br>
>>> >> >> ><br>
>>> >> >> > Currently, I have to do multiple things:<br>
>>> >> >> ><br>
>>> >> >> > 1) create some setLibcallNames in XXXISelLowering.cpp to generate<br>
>>> >> >> > correct<br>
>>> >> >> > naming for RTLIBS.<br>
>>> >> >> > 2) lower ISD down to an RTLIB for some calls (and then do<br>
>>> >> >> > solution 1<br>
>>> >> >> > on<br>
>>> >> >> > those to get correct names)<br>
>>> >> >><br>
>>> >> >> These solve a related but different - CodeGen - problem.<br>
>>> >> >><br>
>>> >> >> RTLIB libcalls are used when we're not able to select some IR<br>
>>> >> >> instruction/intrinsic so have to rely on a runtime library helper<br>
>>> >> >> function (e.g., the stuff in compiler-rt/lib/builtins/).<br>
>>> >> >><br>
>>> >> >> So, #1 and #2 would make LLVM able to emit calls to __xxx_acos when<br>
>>> >> >> it sees "@llvm.acos.f32", but it won't let LLVM optimize (constant<br>
>>> >> >> fold, transform into the intrinsic, ...) "__xx_acos()" when it sees<br>
>>> >> >> it.<br>
>>> >> >><br>
>>> >> >> It sounds like you also want to recognize and optimize these calls.<br>
>>> >> >> That involves (pre-CodeGen) IR-level optimizations.<br>
>>> >> >> No, I don't think that's supported today without changing LLVM (see<br>
>>> >> >> the list in my first email).<br>
>>> >> >><br>
>>> >> >> > 3) change TargetLibraryInfo for functions that aren't covered in<br>
>>> >> >> > solutions 1<br>
>>> >> >> > and 2 (so that they can also be optimized)<br>
>>> >> >> ><br>
>>> >> >> > I must be missing something, I'm just not sure what it is.<br>
>>> >> >> ><br>
>>> >> >> > Thanks,<br>
>>> >> >> ><br>
>>> >> >> > Ryan<br>
>>> >> >> ><br>
>>> >> >> ><br>
>>> >> >> > On Tue, Jun 7, 2016 at 4:45 PM, Ryan Taylor <<a href="mailto:ryta1203@gmail.com">ryta1203@gmail.com</a>><br>
>>> >> >> > wrote:<br>
>>> >> >> >><br>
>>> >> >> >> Tim,<br>
>>> >> >> >><br>
>>> >> >> >> Are you referring to setLibcallName? That is target specific<br>
>>> >> >> >> yes<br>
>>> >> >> >> but<br>
>>> >> >> >> there isn't RTLIB for most of the libm functions, for example,<br>
>>> >> >> >> for<br>
>>> >> >> >> acos<br>
>>> >> >> >> this<br>
>>> >> >> >> doesn't apply.<br>
>>> >> >> >><br>
>>> >> >> >> Ideally what I would like is to create a libc with functions<br>
>>> >> >> >> like<br>
>>> >> >> >> acos<br>
>>> >> >> >> called something like __xxx_acos that can still be recognized to<br>
>>> >> >> >> be<br>
>>> >> >> >> optimized.<br>
>>> >> >> >><br>
>>> >> >> >> RTLIB is pretty limited but it works fine, I can just use<br>
>>> >> >> >> setLibcallName(RTLIB::floor, "__xxx_floor")... but again, the<br>
>>> >> >> >> functions<br>
>>> >> >> >> that<br>
>>> >> >> >> are RTLIB are limited. Using intrinsics make it more difficult<br>
>>> >> >> >> because<br>
>>> >> >> >> then<br>
>>> >> >> >> you have to match the intrinsic (rather than it automatically<br>
>>> >> >> >> generating a<br>
>>> >> >> >> lib call). ISD is just as bad (FCOPYSIGN, FABS for example)<br>
>>> >> >> >> because<br>
>>> >> >> >> then<br>
>>> >> >> >> they need to be manually lowered.<br>
>>> >> >> >> Thanks,<br>
>>> >> >> >><br>
>>> >> >> >> Ryan<br>
>>> >> >> >><br>
>>> >> >> >><br>
>>> >> >> >><br>
>>> >> >> >> On Tue, Jun 7, 2016 at 4:38 PM, Tim Northover<br>
>>> >> >> >> <<a href="mailto:t.p.northover@gmail.com">t.p.northover@gmail.com</a>><br>
>>> >> >> >> wrote:<br>
>>> >> >> >>><br>
>>> >> >> >>> On 7 June 2016 at 13:24, Ryan Taylor via llvm-dev<br>
>>> >> >> >>> <<a href="mailto:llvm-dev@lists.llvm.org">llvm-dev@lists.llvm.org</a>> wrote:<br>
>>> >> >> >>> > Not sure why it's called TargetLibraryInfo if it's not in<br>
>>> >> >> >>> > target<br>
>>> >> >> >>> > specific<br>
>>> >> >> >>> > code? It seems that ALL targets use this code, making it<br>
>>> >> >> >>> > generic.<br>
>>> >> >> >>> > Am<br>
>>> >> >> >>> > I<br>
>>> >> >> >>> > missing something here?<br>
>>> >> >><br>
>>> >> >> I agree the name "Target" is a bit awkward, but it's not generic in<br>
>>> >> >> that it behaves differently depending on the target triple, which<br>
>>> >> >> is<br>
>>> >> >> usually not OK in a "generic" analysis.<br>
>>> >> >><br>
>>> >> >> If you look in TargetLibraryInfo.cpp, there are various checks for<br>
>>> >> >> function availability, usually predicated on OS versions.<br>
>>> >> >><br>
>>> >> >> -Ahmed<br>
>>> >> >><br>
>>> >> >> >>> Some of the names can vary by platform, for example ARM<br>
>>> >> >> >>> sometimes<br>
>>> >> >> >>> has<br>
>>> >> >> >>> __aeabi_memcpy instead of memcpy<br>
>>> >> >> >>><br>
>>> >> >> >>> > ps. The spec also states (albeit unclearly) that you can use<br>
>>> >> >> >>> > "#undef"<br>
>>> >> >> >>> > to<br>
>>> >> >> >>> > omit a library function so that a user defined function of<br>
>>> >> >> >>> > the<br>
>>> >> >> >>> > same<br>
>>> >> >> >>> > name can<br>
>>> >> >> >>> > be used but LLVM doesn't seem to support that.<br>
>>> >> >> >>><br>
>>> >> >> >>> I think it says exactly the opposite: (7.1.2p3):<br>
>>> >> >> >>><br>
>>> >> >> >>> "If the program removes (with #undef) any macro definition<br>
>>> >> >> >>> of<br>
>>> >> >> >>> an<br>
>>> >> >> >>> identifier in the first group listed above, the behavior is<br>
>>> >> >> >>> undefined."<br>
>>> >> >> >>><br>
>>> >> >> >>> Incidentally, I don't think anyone's mentioned that<br>
>>> >> >> >>> "-ffreestanding"<br>
>>> >> >> >>> will probably inhibit the intrinsics substantially if that's<br>
>>> >> >> >>> what<br>
>>> >> >> >>> you're after (technically, it's probably a compiler extension<br>
>>> >> >> >>> that<br>
>>> >> >> >>> it<br>
>>> >> >> >>> gives them back to the user, but everyone does it as far as I<br>
>>> >> >> >>> know).<br>
>>> >> >> >>><br>
>>> >> >> >>> Cheers.<br>
>>> >> >> >>><br>
>>> >> >> >>> Tim.<br>
>>> >> >> >><br>
>>> >> >> >><br>
>>> >> >> ><br>
>>> >> ><br>
>>> >> ><br>
>>> ><br>
>>> ><br>
>><br>
>><br>
><br>
><br>
> _______________________________________________<br>
> LLVM Developers mailing list<br>
> <a href="mailto:llvm-dev@lists.llvm.org">llvm-dev@lists.llvm.org</a><br>
> <a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a><br>
><br>
<br>
<br>
<br>
--<br>
With best regards, Anton Korobeynikov<br>
Department of Statistical Modelling, Saint Petersburg State University<br>
</blockquote></div>