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

Tue Aug 18 17:09:49 PDT 2015

> On Aug 18, 2015, at 4:31 PM, Quentin Colombet via llvm-commits <llvm-commits at lists.llvm.org> wrote:
> 
> Thanks Hal for your responsiveness!
> 
> I’ll revert r221876 and add a test case at the same time for PR24468.

This is r245394 and r245395.


> 
> Q.
>> On Aug 18, 2015, at 4:20 PM, Hal Finkel <hfinkel at anl.gov <mailto:hfinkel at anl.gov>> wrote:
>> 
>> ----- Original Message -----
>>> From: "Quentin Colombet" <qcolombet at apple.com <mailto:qcolombet at apple.com>>
>>> To: reviews+D11847+public+2636e964e792efad at reviews.llvm.org <mailto:reviews+D11847+public+2636e964e792efad at reviews.llvm.org>, "Hal J. Finkel" <hfinkel at anl.gov <mailto:hfinkel at anl.gov>>
>>> Cc: "n j white" <n.j.white at gmail.com <mailto:n.j.white at gmail.com>>, llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>
>>> Sent: Tuesday, August 18, 2015 6:04:44 PM
>>> Subject: Re: [PATCH] D11847: [BasicAA] Fix zext & sext handling
>>> 
>>> 
>>> 
>>> 
>>>> On Aug 18, 2015, at 3:43 PM, hfinkel at anl.gov <mailto:hfinkel at anl.gov> via llvm-commits
>>>> <llvm-commits at lists.llvm.org <mailto: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= <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?
>> 
>> Okay. In that case, let's revert.
>> 
>> -Hal
>> 
>>> 
>>> Thanks,
>>> -Quentin
>>> 
>>> 
>>> 
>>>> 
>>>> 
>>>> 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= <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=
>>> 
>>> 
>> 
>> -- 
>> Hal Finkel
>> Assistant Computational Scientist
>> Leadership Computing Facility
>> Argonne National Laboratory
> 
> _______________________________________________
> 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=Qw7feHjNo6zLdLt1TUaN79trLbYAagCtXu5YPhxJh3Y&s=Ju1l9wdGtrJF1wJ89Qy7UB2Z5i5J8qO4OG85lgohG0Y&e= 

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