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