[llvm] r214011 - [x86] Fix PR20355 (for real). There are many layers to this bug.

[Quentin Colombet]

Thanks Ben!

I’ll see if I can expose the bug with that approach.

-Quentin

> On Aug 5, 2014, at 2:02 AM, Benjamin Kramer <benny.kra at gmail.com> wrote:
> 
> On Mon, Aug 4, 2014 at 7:48 PM, Quentin Colombet <qcolombet at apple.com> wrote:
>> Ping?
>> 
>> Benjamin, any idea on how we can write a test for the lowering of the low
>> part of the MUL_LOHI on x86?
> 
> The only way I can think of is having a wide mul in IR and let the
> legalizer emit a MUL_LOHI. I don't know what magic incantation is
> needed for that though :/
> 
> - Ben
> 
>> 
>> Thanks,
>> -Quentin
>> 
>> On Jul 29, 2014, at 9:22 AM, Quentin Colombet <qcolombet at apple.com> wrote:
>> 
>> 
>> On Jul 28, 2014, at 2:21 PM, Quentin Colombet <qcolombet at apple.com> wrote:
>> 
>> 
>> On Jul 28, 2014, at 12:58 PM, Chandler Carruth <chandlerc at gmail.com> wrote:
>> 
>> 
>> On Mon, Jul 28, 2014 at 12:46 PM, Quentin Colombet <qcolombet at apple.com>
>> wrote:
>>> 
>>> I think we still have a bug in the current implementation.
>>> See my inline comments.
>> 
>> 
>> Seems likely. Can you craft a test case to demonstrate it?
>> 
>> 
>> I will see if I can come up with something.
>> 
>> 
>> I haven’t found anything yet. The case I know that was using this path (div
>> -> mul) just uses the high part of the result. Thus, the problem does not
>> show up.
>> 
>> CC’ed Benjamin, he may have an idea on how to test that.
>> 
>> Cheers,
>> -Quentin
>> 
>> 
>> 
>>> 
>>> 
>>> Also, I think it would be nice to update the comments on the masks value
>>> instead of removing them.
>>> When I reversed engineered this code, it took me ages to figure out what
>>> were these magic values.
>> 
>> 
>> I'm not sure that's going to help. The problem with comments is that they
>> can get out of sync. I also don't think that little endian offsets are
>> surprising or need lots of clarification here? The 1 instead of 0 thing you
>> point out screams wrong to me at a glance, and I don't think a comment would
>> really make it better (or worse, unless it got out of date).
>> 
>> 
>> Fair enough!
>> 
>> -Quentin
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>> 
>> 
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>> 
>>