Tweak ConstantHoisting on X86

Michael Zolotukhin mzolotukhin at apple.com
Wed Apr 30 10:52:09 PDT 2014


Hi Quentin,

Thanks! I think I fixed the test in the second version of the patch, making it independent on constant hoisting. You could find it in later letters in this thread. However, it would be good to check if the original test case behave properly after this change as well.

Michael

On Apr 30, 2014, at 9:46 PM, Quentin Colombet <qcolombet at apple.com> wrote:

> Hi Michael,
> 
> On Apr 30, 2014, at 6:11 AM, Michael Zolotukhin <mzolotukhin at apple.com> wrote:
> 
>> 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?
>> 
>> 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’ll apply your patch locally and I’ll have a look.
> Unfortunately this kind of bugs needs a lot of conditions to trigger, so it is not that surprising that it is fragile :(.
> 
> Thanks,
> -Quentin
>> 
>> <ConstantHoisting-generic.patch><ConstantHoisting-x86.patch>
>> 
>> <test.ll>
>> 
>> Thanks,
>> Michael
>> 
>> 
> 





More information about the llvm-commits mailing list