[llvm-dev] Potential bug in SelectionDAGLegalize::ConvertNodeToLibcall()?

Fri Jan 4 06:05:05 PST 2019

+ Eli Friedman as he often has very insightful comments regarding back end
changes.

On Fri, Jan 4, 2019 at 9:03 AM Nemanja Ivanovic <nemanja.i.ibm at gmail.com>
wrote:

> The changes seem fine to me. I don't think this is excessively intrusive
> and it accomplishes what is needed by targets whose call lowering can
> introduce illegal types.
> Adding Justin Bogner as the owner of SDAG and Hal Finkel as the PPC back
> end owner for their opinions.
>
> Nemanja
>
> On Thu, Jan 3, 2019 at 4:54 PM Justin Hibbits <jrh29 at alumni.cwru.edu>
> wrote:
>
>> Hi Nemanja,
>>
>> I'm attaching a patch that builds on D54583 and implements what we
>> discussed on IRC earlier today.  Particularly:
>>
>> * Make LowerCallTo() a virtual function, so it can be wrapped by a
>>   subclass.
>> * Implement LowerCallTo() in PPCTargetLowering to wrap
>>   TargetLowering::LowerCallTo() and legalize the return node when
>>   targeting SPE.
>> * Augment PPCTargetLowering::LowerCall_32SVR4() to legalize MVT::f64
>>   arguments that have been pre-processed into
>>   EXTRACT_ELEMENT(i64 BITCAST f64, 0/1)
>>
>> The purpose of this being to legalize intermediate illegal types
>> post-type legalization.
>>
>> Is there a better approach?  Comments from anyone else?
>>
>> - Justin
>>
>> On Wed, 2 Jan 2019 11:39:59 -0500
>> Nemanja Ivanovic <nemanja.i.ibm at gmail.com> wrote:
>>
>> > It sounds like the legalizer is lowering `fmaxnum` to a libcall
>> > because it is not a legal node for `f64` and in doing so, it is
>> > producing the `build_pair` to reassemble the results of the libcall.
>> > And presumably, it is assuming that the new nodes do not need
>> > legalization or something along those lines.
>> >
>> > Justin, it would probably be good if you could provide the DAG before
>> > and after legalization both with and without your patch. Then we can
>> > see how it was handled before your patch and how it is handled now
>> > and the difference should allude to the problem.
>> >
>> > N
>> >
>> > On Wed, Jan 2, 2019 at 10:54 AM Justin Hibbits via llvm-dev <
>> > llvm-dev at lists.llvm.org> wrote:
>> >
>> > > Hi,
>> > >
>> > > I have a custom lowering operation on ISD::BITCAST for the
>> > > PowerPC/SPE target, to convert 'f64 bitcast (i64 build_pair i32,
>> > > i32)' into a 'f64 BUILD_SPE64 i32, i32' node, which can be seen at
>> > > https://reviews.llvm.org/D54583. However, when building
>> > > compiler-rt's lib/builtins/divdc3.c an assertion is triggered that
>> > > BUILD_PAIR is not legal on line 24.  There should be no
>> > > bitcast(buildpair) anywhere in the generation, as it should all be
>> > > lowered.  However, this is not the case when lowering to a libcall
>> > > it seems.  The relevant debug output, is here:
>> > >
>> > > Creating new node: t118: i64 = build_pair t117,t116,
>> > > /home/chmeee/freebsd/contrib/compiler-rt/lib/builtins/divdc3.c:24:22
>> > > Creating new node: t119: f64 = bitcast t118,
>> > > /home/chmeee/freebsd/contrib/compiler-rt/lib/builtins/divdc3.c:24:22
>> > > Created libcall: t119: f64 = bitcast t118,
>> > > /home/chmeee/freebsd/contrib/compiler-rt/lib/builtins/divdc3.c:24:22
>> > > Successfully converted node to libcall
>> > >  ... replacing: t38: f64 = fmaxnum t36, t37,
>> > > /home/chmeee/freebsd/contrib/compiler-rt/lib/builtins/divdc3.c:24:22
>> > >      with:      t119: f64 = bitcast t118,
>> > > /home/chmeee/freebsd/contrib/compiler-rt/lib/builtins/divdc3.c:24:22
>> > >
>> > > Is this a real bug, or am I missing something in my patch?  After
>> > > spending quite a while on it I'm at a loss.
>> > >
>> > > Thanks,
>> > > Justin
>> > > _______________________________________________
>> > > LLVM Developers mailing list
>> > > llvm-dev at lists.llvm.org
>> > > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>> > >
>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20190104/54f2fe31/attachment-0001.html>