[PATCH] fix PR15296

Nadav Rotem nrotem at apple.com
Wed Feb 20 10:13:06 PST 2013


LGTM. 

On Feb 20, 2013, at 10:11 AM, Michael Liao <michael.liao at intel.com> wrote:

> Hi Nadav
> 
> On Wed, 2013-02-20 at 09:27 -0800, Nadav Rotem wrote:
>> Patch #1:
>> 
>> +SDValue X86TargetLowering::LowerShift(SDValue Op, SelectionDAG &DAG) const {
>> +
>> +  EVT VT = Op.getValueType();
>> 
>> No need for extra space. 
> 
> Will fix that.
> 
>> 
>> Patch #2: 
>> 
>> Please write a few cost-model tests to make sure that we get the costs of the shifts right. 
> 
> Sure.
> 
>> 
>> Patch #3:
>> 
>> In this patch you are doing two things. 1. Move code from x86combine to x86lower, and add a new optimization. Please split this into two patches.
> 
> The patch is mostly moving. The code added otherwise to handle 64-bit
> shift in 32-bit mode as DAG combining won't see the extended 64-bit
> type. Unless we disable some test cases, splitting this patch will break
> builds.
> 
> - michael
> 
>> 
>> Thanks,
>> Nadav  
>> 
>> 
> 
> 




More information about the llvm-commits mailing list