[llvm] r207616 - Use makeArrayRef insted of calling ArrayRef<T> constructor directly. I introduced most of these recently.

[David Blaikie]

On Wed, Apr 30, 2014 at 6:10 PM, Craig Topper <craig.topper at gmail.com> wrote:
>>
>> We should probably have an "ops" that returns ArrayRef (ArrayRef being
>> the iterator_range for contiguous in-memory sequences - and handy for
>> other things).
>
>
> Agreed. It came up in a few places. I'll look into doing that.
>
>>
>>
>> > Modified: llvm/trunk/lib/Target/Mips/MipsSEISelLowering.cpp
>> > URL:
>> > http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/Mips/MipsSEISelLowering.cpp?rev=207616&r1=207615&r2=207616&view=diff
>> >
>> > ==============================================================================
>> > --- llvm/trunk/lib/Target/Mips/MipsSEISelLowering.cpp (original)
>> > +++ llvm/trunk/lib/Target/Mips/MipsSEISelLowering.cpp Wed Apr 30
>> > 02:17:30 2014
>> > @@ -489,7 +489,7 @@ static SDValue performANDCombine(SDNode
>> >        SDValue Ops[] = { Op0->getOperand(0), Op0->getOperand(1), Op0Op2
>> > };
>> >        DAG.MorphNodeTo(Op0.getNode(), MipsISD::VEXTRACT_ZEXT_ELT,
>> >                        Op0->getVTList(),
>> > -                      ArrayRef<SDValue>(Ops, Op0->getNumOperands()));
>> > +                      makeArrayRef(Ops, Op0->getNumOperands()));
>>
>> Any idea what getNumOperands returns? Can it really return various
>> values <= 3? (can't be more than 3, since Ops only has 3 things). If
>> it's always 3, then you can just drop the makeArrayRef call entirely
>> and rely on the implicit conversion from array to ArrayRef.
>
>
> It wasn't immediately obvious to me when I did the ArrayRef conversions
> which is why I made it explicit, but I think it might be 3.

If I'm feeling particularly aggressive I'll try throwing an assert it
& rebuild/test, see if anything turns up (& sometimes, if nothing
does, just commit it & wait to see if anyone complains) - but yeah,
depends on the scope/confidence/etc.

>> (similar comment for the next 3 changes \/)
>>
>> >        return Op0;
>> >      }
>> >    }
>> > @@ -834,7 +834,7 @@ static SDValue performSRACombine(SDNode
>> >                            Op0Op0->getOperand(2) };
>> >          DAG.MorphNodeTo(Op0Op0.getNode(), MipsISD::VEXTRACT_SEXT_ELT,
>> >                          Op0Op0->getVTList(),
>> > -                        ArrayRef<SDValue>(Ops,
>> > Op0Op0->getNumOperands()));
>> > +                        makeArrayRef(Ops, Op0Op0->getNumOperands()));
>> >          return Op0Op0;
>> >        }
>> >      }
>> > @@ -1284,7 +1284,7 @@ static SDValue lowerMSASplatZExt(SDValue
>> >                        LaneA, LaneB, LaneA, LaneB, LaneA, LaneB, LaneA,
>> > LaneB };
>> >
>> >    SDValue Result = DAG.getNode(ISD::BUILD_VECTOR, DL, ViaVecTy,
>> > -                       ArrayRef<SDValue>(Ops,
>> > ViaVecTy.getVectorNumElements()));
>> > +                       makeArrayRef(Ops,
>> > ViaVecTy.getVectorNumElements()));
>> >
>> >    if (ViaVecTy != ResVecTy)
>> >      Result = DAG.getNode(ISD::BITCAST, DL, ResVecTy, Result);
>> > @@ -1324,7 +1324,7 @@ static SDValue getBuildVectorSplat(EVT V
>> >                        SplatValueA, SplatValueB, SplatValueA,
>> > SplatValueB };
>> >
>> >    SDValue Result = DAG.getNode(ISD::BUILD_VECTOR, DL, ViaVecTy,
>> > -                       ArrayRef<SDValue>(Ops,
>> > ViaVecTy.getVectorNumElements()));
>> > +                       makeArrayRef(Ops,
>> > ViaVecTy.getVectorNumElements()));
>> >
>> >    if (VecTy != ViaVecTy)
>> >      Result = DAG.getNode(ISD::BITCAST, DL, VecTy, Result);
>>
>
> I think for at least the last two, there is a static array sized to the
> maximum vector size and the ViaVecTy.getVectorNumElements() determines how
> much of it is used. Could probably create a SmallVector there and explicitly
> push to the correct size.

Perhaps  - I'm not too fussed either way there.

- David