[LLVMdev] MachineSink and EFLAGS

[Galanov, Sergey]

Hi Bill.

Thank you very much! Now I see my understanding was incorrect :) A dependence from a single physreg-defining instruction (like CMP or TEST) is allowed to be shared in several instructions unless that register is not clobbered (and this is what we have with CMOV_FR64). Wouldn't it be safe then to not set the live-in flag in EmitLoweredSelect for instructions which are marked as defining EFLAGS (like the integer pseudo cmovs)?

Thanks,
Sergey

-----Original Message-----
From: Bill Wendling [mailto:wendling at apple.com] 
Sent: Thursday, June 02, 2011 12:00 AM
To: Galanov, Sergey
Cc: llvmdev at cs.uiuc.edu
Subject: Re: [LLVMdev] MachineSink and EFLAGS

On Jun 1, 2011, at 9:18 AM, Galanov, Sergey wrote:

> Hello.
>  
> I am not sure this is the right place to ask but here is my question. 
> About a year ago there was a fix of some obscure bug (rdar://problem/8030636 which is located on the internal Apple bugtracker I believe and so not available to the general public J) Some discussion can be found here: http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20100531/102160.html. Unfortunately, no real testcase is provided, just abstract scenario description.
>  
> Basically a EFLAGS-clobbering instruction is not sunk if EFLAGS might be live out of the current block and since a conditional branch doesn't mark its EFLAGS use as a kill, that effectively means it is never sunk. But how can that happen? I think the only way for it is when EFLAGS def and use are located in different blocks but that is impossible with current instruction selector. Or am I wrong?
>  
> A little later another fix has been made (http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20100614/102554.html) which restricts the problem only to cases when the blocks are produced by lowering a SELECT instruction. It is even more confusing. How is it different from the other scenario wrt. EFLAGS use? Moreover, pseudo cmov is marked as clobbering EFLAGS so how can it be used further in the code?
>  
> Are there plans to implement a less conservative fix? I believe proper phys regs liveness information is required for that.
>  
Hi Sergey,

I was the one who created the patch. Unfortunately, there wasn't a testcase that I could use that was small and wouldn't require modification after every compiler change.

Looking at the bug report, I've attached the testcase below. You can compile it like this:

$ llvm-gcc -Os -fmessage-length=0 -pipe -std=gnu99 -fpascal-strings -fasm-blocks -pedantic -msse3 test.c -o test.s -S

See the attached test.s for comments on what is wrong with the output.

You're correct that proper physical register liveness information is required for a less conservative fix. However, I don't know if that will ever happen. If it does, then we can make this pass more liberal.

-bw


--------------------------------------------------------------------
Closed Joint Stock Company Intel A/O
Registered legal address: Krylatsky Hills Business Park, 
17 Krylatskaya Str., Bldg 4, Moscow 121614, 
Russian Federation

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.