Tweak ConstantHoisting on X86

[Juergen Ributzka]

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/5977caee/attachment.html>