[llvm-commits] [PATCH 2/2] FIx PR12312
Evan Cheng
evan.cheng at apple.com
Tue Aug 28 11:53:00 PDT 2012
Sorry, more nitpick:
+ Opnds.push_back(I->getOperand(0));
+ Opnds.push_back(I->getOperand(1));
+ // Re-evaluate the number of nodes to be traversed.
+ e = Opnds.size();
Can't you replace it with?
e += 2;
Evan
On Aug 27, 2012, at 3:38 PM, Michael Liao <michael.liao at intel.com> wrote:
> On Mon, 2012-08-27 at 14:39 -0700, Evan Cheng wrote:
>> Sorry, just got back.
>>
>> One nitpick:
>>
>> for (unsigned Slot = 0; Slot < Opnds.size(); ++Slot) {
>>
>> You want something like this to avoid calling Opnds.size() in every iteration.
>> for (unsigned Slot = 0, e = Opnds.size(); Slot < e; ++Slot) {
>
> As the loop body will append elements into Opnds, this will quit the
> loop earlier than expectation. To avoid per-iteration Opnds.size(), I
> need extra 'e' updating once this small vector is updated. Here is the
> revised patch.
>
> Yours
> - Michael
>
>>
>> Otherwise LGTM.
>>
>> Evan
>>
>> On Aug 18, 2012, at 9:15 PM, Michael Liao <michael.liao at intel.com> wrote:
>>
>>> ping again. - Michael
>>>
>>> On Tue, 2012-08-14 at 23:49 -0700, Michael Liao wrote:
>>>> Here is the revised patch putting 128-bit vector checking earlier.
>>>>
>>>> Yours
>>>> - Michael
>>>>
>>>> On Tue, 2012-08-14 at 13:53 -0700, Evan Cheng wrote:
>>>>> + if (VT.is128BitVector())
>>>>> + VecIn = DAG.getNode(ISD::BITCAST, DL, MVT::v2i64, VecIn);
>>>>> + else // FIXME: only 128-bit vector is supported so far.
>>>>> + return SDValue();
>>>>>
>>>>> It seems like this check can happen earlier to avoid doing a bunch of work for no good reason.
>>>>>
>>>>> Evan
>>>>>
>>>>> On Aug 13, 2012, at 1:25 PM, Michael Liao <michael.liao at intel.com> wrote:
>>>>>
>>>>>> Here is another revision of the 2nd part. The new change fixes an
>>>>>> potential issue of stale iterator being used. The new loop BFS
>>>>>> traversing OR'd tree is now over index instead of iterator as we may add
>>>>>> elements in that loop.
>>>>>>
>>>>>> Yours
>>>>>> - Michael
>>>>>>
>>>>>> On Sun, 2012-08-12 at 14:28 -0700, Michael Liao wrote:
>>>>>>> On Fri, 2012-08-10 at 14:27 -0700, Evan Cheng wrote:
>>>>>>>> +static SDValue FlaggedOrCombine(SDValue Or, X86::CondCode &CC, SelectionDAG &DAG,
>>>>>>>> + const X86Subtarget *Subtarget) {
>>>>>>>>
>>>>>>>>
>>>>>>>> The current naming convention calls for lower cased first letter.
>>>>>>>
>>>>>>> The revised patch is attached. I checked the function naming in the 1st
>>>>>>> part of patch as well.
>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> + if (VecIn != DAG.getVectorShuffle(VT, DL, VecIn, VecIn, &Mask[0]))
>>>>>>>> + return SDValue();
>>>>>>>>
>>>>>>>> What is this trying to do? Is it potentially creating a vectorshuffle node and then throwing it away? Please don't do that.
>>>>>>>>
>>>>>>>
>>>>>>> It is checking whether the all vector elements are used in that OR'd
>>>>>>> tree. I revised the code to use a bit mask for this purpose and simplify
>>>>>>> both this checking and other parts.
>>>>>>>
>>>>>>> Please review the revised patch attached.
>>>>>>>
>>>>>>> Yours
>>>>>>> - Michael
>>>>>>>
>>>>>>>> Evan
>>>>>>>>
>>>>>>>> On Aug 1, 2012, at 12:47 PM, Michael Liao <michael.liao at intel.com> wrote:
>>>>>>>>
>>>>>>>>> Hi
>>>>>>>>>
>>>>>>>>> This patch is to fix http://llvm.org/bugs/show_bug.cgi?id=12312, where a
>>>>>>>>> special use of i128 needs efficient code generation with PTEST from
>>>>>>>>> SSE4.1.
>>>>>>>>>
>>>>>>>>> To fix this issue, 2 patches are developed. The second part is to
>>>>>>>>> recognize that special use, casting from <4 x i32> to i128 followed by a
>>>>>>>>> zero (or non-zero) checking. That sequence is lowered into OR
>>>>>>>>> (X86ISD::OR at the root) tree with all leaves extracted from that vector
>>>>>>>>> because i128 is mapped onto GPR pair. The attached patch adds a
>>>>>>>>> X86-specific DAG optimization to recognize this OR'd tree and convert it
>>>>>>>>> a PTEST if only the flag value is used from that X86ISD::OR root node.
>>>>>>>>>
>>>>>>>>> Yours
>>>>>>>>> - Michael
>>>>>>>>> <0002-add-X86-DAG-optimization-to-recognize-a-pttern-PTEST.patch>_______________________________________________
>>>>>>>>> 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
>>>>>>
>>>>>> <0001-add-X86-DAG-optimization-to-recognize-a-pttern-PTEST.patch>
>>>>>
>>>>
>>>> _______________________________________________
>>>> llvm-commits mailing list
>>>> llvm-commits at cs.uiuc.edu
>>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>>
>>>
>>
>
> <0001-add-X86-DAG-optimization-to-recognize-a-pttern-PTEST.patch>
More information about the llvm-commits
mailing list