[PATCH] D11847: [BasicAA] Fix zext & sext handling

Quentin Colombet via llvm-commits llvm-commits at lists.llvm.org
Tue Aug 18 16:04:44 PDT 2015


> On Aug 18, 2015, at 3:43 PM, hfinkel at anl.gov via llvm-commits <llvm-commits at lists.llvm.org> wrote:
> 
> hfinkel added a comment.
> 
> In https://urldefense.proofpoint.com/v2/url?u=http-3A__reviews.llvm.org_D11847-23227079&d=BQIGaQ&c=eEvniauFctOgLOKGJOplqw&r=Kar_gr4lUTX9iRgFLHyIPCR7VD3VlB3-02h_Q5v9oWk&m=MpOmT90EJ-G0_kGQEMbywvqzTIbeadHteqN7q7h7Mn0&s=Jyz-iD2ioHwmBYyshZdhYpv4WHXfL-BZ8AFI6aG1DQc&e= , @hans wrote:
> 
>> In https://urldefense.proofpoint.com/v2/url?u=http-3A__reviews.llvm.org_D11847-23227064&d=BQIGaQ&c=eEvniauFctOgLOKGJOplqw&r=Kar_gr4lUTX9iRgFLHyIPCR7VD3VlB3-02h_Q5v9oWk&m=MpOmT90EJ-G0_kGQEMbywvqzTIbeadHteqN7q7h7Mn0&s=jbagtOjQ81ZE2_2Ad6QWvHAVMFDYkZmkNBTZ5R-HQWw&e= , @hfinkel wrote:
>> 
>>> In https://urldefense.proofpoint.com/v2/url?u=http-3A__reviews.llvm.org_D11847-23227037&d=BQIGaQ&c=eEvniauFctOgLOKGJOplqw&r=Kar_gr4lUTX9iRgFLHyIPCR7VD3VlB3-02h_Q5v9oWk&m=MpOmT90EJ-G0_kGQEMbywvqzTIbeadHteqN7q7h7Mn0&s=onbkCh-aT1_E8mOtBOsk10wWGw5dtFhQvR9FGrC3LgU&e= , @hans wrote:
>>> 
>>>> Did this get committed? Is this a fix we need for 3.7?
>>> 
>>> 
>>> From what qcolombet mentioned, this fixes a bug introduced in r221876. That would make it nice to have in 3.7, and I'm fine with it being committed to trunk and making my requested commentary improvements in follow-up. However, this is not purely a bug fix, and furthermore, this fix itself has been committed/reverted before as well. I'm nervous about merging it into the release branch late in the release cycle. It might be best to revert r221876 in the release branch instead. Is there going to be another release candidate at this point before 3.7?
>> 
>> 
>> r221876 also doesn't look like a trivial patch. It's hard to follow along with all the reverted reverts here.
>> 
>> From what I understand, PR23626 that's mentioned here was already fixed by reverting something else on 3.7, so maybe we don't need this or reverting r221876?
>> 
>> There have been a number of merges after 3.7-rc2, so there will be an rc3, but I'm hoping that will become the final one.
> 
> 
> Okay; I'll make sure this gets committed today.

We need to hold on this fix/improvement.
I did some more testing with that patch and it turns out it breaks a lot of tests in the LLVM test suite.

For instance, on x86_64 with Os, try SingleSource/Benchmarks/Shoutout/heapsort.

I think at this point it is best to revert r221876 and have a step back on the whole design of this improvement.

Attached the IR output for heap sort with and without the patch.

What do you think?

Thanks,
-Quentin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: with.ll
Type: application/octet-stream
Size: 8133 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150818/b4cdeb4b/attachment.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: without.ll
Type: application/octet-stream
Size: 9043 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150818/b4cdeb4b/attachment-0001.obj>
-------------- next part --------------

> 
> 
> https://urldefense.proofpoint.com/v2/url?u=http-3A__reviews.llvm.org_D11847&d=BQIGaQ&c=eEvniauFctOgLOKGJOplqw&r=Kar_gr4lUTX9iRgFLHyIPCR7VD3VlB3-02h_Q5v9oWk&m=MpOmT90EJ-G0_kGQEMbywvqzTIbeadHteqN7q7h7Mn0&s=4FRBNH1J1Nrf4m77QozOqM-iJHGyXuqpxhgwluZDjcY&e= 
> 
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> https://urldefense.proofpoint.com/v2/url?u=http-3A__lists.llvm.org_cgi-2Dbin_mailman_listinfo_llvm-2Dcommits&d=BQIGaQ&c=eEvniauFctOgLOKGJOplqw&r=Kar_gr4lUTX9iRgFLHyIPCR7VD3VlB3-02h_Q5v9oWk&m=MpOmT90EJ-G0_kGQEMbywvqzTIbeadHteqN7q7h7Mn0&s=SMp__3HeFsLsXLuFxQwIe4BmvifY1hwXW_ejIjnQe8I&e= 



More information about the llvm-commits mailing list