Tweak ConstantHoisting on X86

[Michael Zolotukhin]

Thanks, Juergen!

Before check-in I’m going to run some additional testing and wait for an update from Quentin (and maybe comments from others).

Michael

On Apr 30, 2014, at 9:13 PM, Juergen Ributzka <juergen at apple.com> wrote:

> Thanks for fixing this Michael. LGTM
> -Juergen
> On Apr 30, 2014, at 7:49 AM, Michael Zolotukhin <mzolotukhin at apple.com> wrote:
> 
>> 
>> On Apr 30, 2014, at 5:17 PM, Hal Finkel <hfinkel at anl.gov> wrote:
>> 
>>> ----- Original Message -----
>>>> From: "Michael Zolotukhin" <mzolotukhin at apple.com>
>>>> To: "llvm commits" <llvm-commits at cs.uiuc.edu>
>>>> Sent: Wednesday, April 30, 2014 8:11:16 AM
>>>> Subject: Tweak ConstantHoisting on X86
>>>> 
>>>> 
>>>> 
>>>> Hi,
>>>> 
>>>> ConstantHoisting currently could lead to inefficient code on X86 (the
>>>> test is attached). Recently Juergen has solved a similar issue on
>>>> ARM64, so these changes do the same on X86. The change is to
>>>> disallow hoisting of constants from shift instructions.
>>>> 
>>>> While the original issue that I’m fixing is on X86, the same problem
>>>> could happen on other platforms. So, I prepared two versions of the
>>>> patch: the first one touches only X86, and the other changes
>>>> ConstantHoisting itself. Personally, I’d prefer the X86-only
>>>> version, because I don’t know much about other targets affected. Any
>>>> thoughts on this?
>>> 
>>> Yes, please use the x86-only version. I would not want exactly this behavior for PowerPC, for example.
>> Sure.
>> 
>>> -Hal
>>> 
>>>> 
>>>> There is one more important detail. With this change test
>>>> CodeGen/X86/remat-invalid-liveness.ll stops to check what it’s
>>>> supposed to check. Right now I just silenced it, but we probably
>>>> want to rewrite and make it less fragile. Quentin, could you please
>>>> take a look and help me with that?
>> I’ve found a simple way to fix the test - as the current change prevents constants from hoisting, I did it in the test manually. Now it checks what it checked before. Also to make sure it’s still relevant I checked that it fails if r206060 is reverted.
>> 
>> Here is an updated patch:
>> <ConstantHoisting-x86-v2.patch>
>> 
>> Is it ok to commit?
>> 
>> Thanks,
>> Michael
>> 
>>>> 
>>>> Thanks,
>>>> Michael
>>>> 
>>>> 
>>>> 
>>>> 
>>>> _______________________________________________
>>>> llvm-commits mailing list
>>>> llvm-commits at cs.uiuc.edu
>>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>>> 
>>> 
>>> -- 
>>> Hal Finkel
>>> Assistant Computational Scientist
>>> Leadership Computing Facility
>>> Argonne National Laboratory
>> 
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140430/e60a3ff1/attachment.html>