On Mon, 3 Sep 2018 at 13:31, Tim Northover <t.p.northover at gmail.com> wrote:> So the next step is to debug where Mips is producing those TruncIntFP > nodes. There'll be some constraint it's not checking or an unexpected > node type, probably related to -msingle-float. I'm afraid I'm not sure > what yet. >I'm reasonably sure the function producing that node is lowerFP_TO_SINT_STORE in lib/Target/Mips/MipsISelLowering.cpp. The node before that function executes has an fp_to_sint node which seems to want to convert an i64 to an f32 which seems...rather odd to me, honestly. The PS2, for what it's worth, only has an i32 -> f32 instruction, so I think there's an impedance mismatch somewhere. -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20180906/c3ca7ed0/attachment.html>
On Thu, 6 Sep 2018 at 16:00, Dan Ravensloft <dan.ravensloft at gmail.com> wrote:> I'm reasonably sure the function producing that node is lowerFP_TO_SINT_STORE in lib/Target/Mips/MipsISelLowering.cpp.That doesn't surprise me.> The node before that function executes has an fp_to_sint node which seems to want to convert an i64 to an f32 which seems...rather odd to me, honestly.It's a pretty common kind of operation. C says you're allowed to cast an int64_t to a float, and that's the IR you get when you try.> The PS2, for what it's worth, only has an i32 -> f32 instruction, so I think there's an impedance mismatch somewhere.This is also a fairly common situation. If the operation can be emulated with a reasonably small number of native instructions you can often get LLVM to do that. In this case it would probably be a libcall though because it's quite complex. LLVM would generate a call to __floatdisf, which will be provided by compiler-rt (there are C implementations for all kinds of floating-point operations there). You should see the same thing if you compile a function doing that conversion with GCC. Cheers. Tim.
> On 6 Sep 2018, at 08:00, Dan Ravensloft via llvm-dev <llvm-dev at lists.llvm.org> wrote: > > On Mon, 3 Sep 2018 at 13:31, Tim Northover <t.p.northover at gmail.com <mailto:t.p.northover at gmail.com>> wrote: > So the next step is to debug where Mips is producing those TruncIntFP > nodes. There'll be some constraint it's not checking or an unexpected > node type, probably related to -msingle-float. I'm afraid I'm not sure > what yet.FWIW, I'm not sure how well tested -msingle-float was on MIPS. I don't think we had any bots testing it.> I'm reasonably sure the function producing that node is lowerFP_TO_SINT_STORE in lib/Target/Mips/MipsISelLowering.cpp. > > The node before that function executes has an fp_to_sint node which seems to want to convert an i64 to an f32 which seems...rather odd to me, honestly. The PS2, for what it's worth, only has an i32 -> f32 instruction, so I think there's an impedance mismatch somewhere.Did you mean those types to be the other way around? fp_to_sint is supposed to take a floating point type and produce an integer type so if you're seeing them backwards like this then that would definitely be a bug. If you're referring to the size mismatch though, that's ok within the IR and DAG nodes. There's no relationship between the size of the input and output.> _______________________________________________ > 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/20180906/689392b4/attachment.html>
On Thu, 6 Sep 2018, 16:35 Daniel Sanders, <daniel_l_sanders at apple.com> wrote:> > > On 6 Sep 2018, at 08:00, Dan Ravensloft via llvm-dev < > llvm-dev at lists.llvm.org> wrote: > > On Mon, 3 Sep 2018 at 13:31, Tim Northover <t.p.northover at gmail.com> > wrote: > >> So the next step is to debug where Mips is producing those TruncIntFP >> nodes. There'll be some constraint it's not checking or an unexpected >> node type, probably related to -msingle-float. I'm afraid I'm not sure >> what yet. > > > FWIW, I'm not sure how well tested -msingle-float was on MIPS. I don't > think we had any bots testing it. >Well, now we have hardware, if all of one machine, where it's worth testing. I'm happy to try it myself if needed, though rigging it up to happen automatically would be difficult.> I'm reasonably sure the function producing that node > is lowerFP_TO_SINT_STORE in lib/Target/Mips/MipsISelLowering.cpp. > > The node before that function executes has an fp_to_sint node which seems > to want to convert an i64 to an f32 which seems...rather odd to me, > honestly. The PS2, for what it's worth, only has an i32 -> f32 instruction, > so I think there's an impedance mismatch somewhere. > > > Did you mean those types to be the other way around? fp_to_sint is > supposed to take a floating point type and produce an integer type so if > you're seeing them backwards like this then that would definitely be a bug. >Yes, my mistake; I got confused by the direction of the arrows on the graph, they're pointing to their parents, but I thought they were pointing to their children.> If you're referring to the size mismatch though, that's ok within the IR > and DAG nodes. There's no relationship between the size of the input and > output. > > _______________________________________________ > 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/20180906/6e064ffc/attachment.html>
On Thu, 6 Sep 2018, 16:31 Tim Northover, <t.p.northover at gmail.com> wrote:> > The PS2, for what it's worth, only has an i32 -> f32 instruction, so I > think there's an impedance mismatch somewhere. > > This is also a fairly common situation. If the operation can be > emulated with a reasonably small number of native instructions you can > often get LLVM to do that. > > In this case it would probably be a libcall though because it's quite > complex. LLVM would generate a call to __floatdisf, which will be > provided by compiler-rt (there are C implementations for all kinds of > floating-point operations there). > > You should see the same thing if you compile a function doing that > conversion with GCC. > > Cheers. > > Tim. >So I was rereading this; do you think the lowering function should instead emit a library call instead, then? If so, would you mind pointing me to a function which performs this, or otherwise give a high-level description of how this is done?>-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20180906/7642f73c/attachment.html>