llvm dev - Jan 2012 - [LLVMdev] Best way to interface with MSVC _ftol2 runtime function for fptoui?

On Jan 24, 2012, at 2:30 PM, Joe Groff wrote:
> On Fri, Jan 20, 2012 at 2:10 PM, Jakob Stoklund Olesen <stoklund at
2pi.dk> wrote:
>> X86FloatingPoint.cpp with comments is all you get.
> 
> Thanks for your help, Jakob. Attached is a first-pass attempt at a
> patch. I don't want to post to -commits yet because I have no idea if
> this is fully correct, but it seems to work in simple test cases. Am I
> on the right track?
Yes, your definition of the new instruction looks sane.

However, you shouldn't expand the instruction right away in
EmitInstrWithCustomInserter(), and leaving the pseudo and call instructions side
by side is not going to work.

Just leave the pseudo-instruction alone until it hits X86FloatingPoint, where
you can rewrite it.
> Could this patch ever break in cases where the
> operand's vreg doesn't happen to get mapped to ST0?
Yes, exactly. You need to make some more complicated test cases.
> I'm still a bit
> foggy on the internals of X86FloatingPoint.
Look at the code handling INLINE_ASM. You need to do the same, except you have
fixed arguments STUses=1 and STClobbers=1, ST*=0. That should greatly simplify
the code you need.
> One thing I noticed is that fptosi and fptoui both seem to always emit
> a redundant SSE load/store when SSE is enabled, because of the check
> at Target/X86/X86ISelLowering.cpp:7948. Can this check be easily
> modified so it doesn't store if the operand is already in memory and
> not actually in an SSE register? Should FP_TO_INTHelper switch over to
> using CVTTS?2SI insns when SSE is available?
When SSE is available, x87 registers are only ever used for f80.

/jakob

On Tue, Jan 24, 2012 at 4:32 PM, Jakob Stoklund Olesen <stoklund at
2pi.dk> wrote:
> Yes, your definition of the new instruction looks sane.
>
> However, you shouldn't expand the instruction right away in
EmitInstrWithCustomInserter(), and leaving the pseudo and call instructions side
by side is not going to work.
>
> Just leave the pseudo-instruction alone until it hits X86FloatingPoint,
where you can rewrite it.
>
> Look at the code handling INLINE_ASM. You need to do the same, except you
have fixed arguments STUses=1 and STClobbers=1, ST*=0. That should greatly
simplify the code you need.
That makes sense; thanks for the tip. Are the getCopyToReg(ST0) and
addReg(ST0, ImplicitKill) calls on the expanded MI at all necessary
then since X86FloatingPoint seems to manage that all internally?
> When SSE is available, x87 registers are only ever used for f80.
It looks like it always tries to use fisttp when converting to i64.
This bitcode:

define i64 @foo(double %x) nounwind readnone {
init:
  %0 = fptosi double %x to i64
  ret i64 %0
}

gets compiled by LLVM 3.0 to:

_foo:                                   # @foo
# BB#0:                                 # %init
	subl	$20, %esp
	movsd	24(%esp), %xmm0
	movsd	%xmm0, 8(%esp)
	fldl	8(%esp)
	fisttpll	(%esp)
	movl	(%esp), %eax
	movl	4(%esp), %edx
	addl	$20, %esp
	ret

with a seemingly redundant movsd pair before the fisttp instruction.

-Joe

Here is a patch that integrates the WIN_FTOL pseudo-insn a bit better
with the floating-point register allocator by following a simplified
version of what InlineAsm does, as you suggested. Does this look
closer to being right? Thanks again for all your help.

-Joe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: llvm-ftol2-2.diff
Type: application/octet-stream
Size: 13739 bytes
Desc: not available
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20120124/ee888a72/attachment.obj>