[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