[PATCH] Generalize getExtendAddRecStart to work with both sign and zero extensions.

Andrew Trick atrick at apple.com
Tue Feb 17 23:18:07 PST 2015


> On Feb 17, 2015, at 11:11 PM, Sanjoy Das <sanjoy at playingwithpointers.com> wrote:
> 
> Looks like I misinterpreted your review comment. :)  Does what I
> checked in look okay though?

Yes. It’s a matter of style and at your discretion if you want to keep the base class, remove it, or do something more clever.

If you keep it, do please fix this:

- ExtendOpTraits<SCEVSignExtendExpr>::GetExtendExprTy
+ ExtendOpTraitsBase::GetExtendExprTy

Incidentally, I left out the typedefs in my code snippet below:

typedef const SCEV *(*GetExtendExprTy)(const SCEV *, Type *);
typedef const SCEV *(*GetOverflowLimitForStepTy)(const SCEV *,
                                                 ICmpInst::Predicate *,
                                                 ScalarEvolution *);

-Andy

> 
> -- Sanjoy
> 
> 
> On Tue, Feb 17, 2015 at 11:00 PM, Andrew Trick <atrick at apple.com> wrote:
>> I can see how my comment about duplicating the typedef would seem silly. I was actually thinking about bringing the members defined by that type into the base, as shown below. But I didn't think it through because now that I write it out, the out-of-line static definitions are utterly horrid:
>> 
>> template <SCEV::NoWrapFlags W, GetExtendExprTy ExtendF,
>> 
>>  GetOverflowLimitForStepTy OverflowF>
>> 
>> struct ExtendOpTraitsBase {
>> 
>>  static const SCEV::NoWrapFlags WrapType = W;
>>  static const GetExtendExprTy GetExtendExpr;
>>  static const GetOverflowLimitForStepTy GetOverflowLimitForStep;
>> 
>> };
>> 
>> template <SCEV::NoWrapFlags W, GetExtendExprTy ExtendF,
>> 
>>  GetOverflowLimitForStepTy OverflowF>
>> 
>> const GetExtendExprTy ExtendOpTraitsBase<W, ExtendF, OverflowF>::
>> GetExtendExpr = ExtendF;
>> 
>> template <SCEV::NoWrapFlags W, GetExtendExprTy ExtendF,
>> 
>>  GetOverflowLimitForStepTy OverflowF>
>> 
>> const GetOverflowLimitForStepTy ExtendOpTraitsBase<W, ExtendF, OverflowF>::
>> GetOverflowLimitForStep = OverflowF;
>> 
>> template <typename ExtendOp> struct ExtendOpTraits {};
>> 
>> template <>
>> struct ExtendOpTraits<SCEVSignExtendExpr>
>> 
>>  : public ExtendOpTraitsBase<SCEV::FlagNSW, getSignExtendExpr,
>>                              getSignedOverflowLimitForStep>
>> 
>> {};
>> 
>> template <>
>> struct ExtendOpTraits<SCEVZeroExtendExpr>
>> 
>>  : public ExtendOpTraitsBase<SCEV::FlagNUW, getZeroExtendExpr,
>>                              getUnsignedOverflowLimitForStep>
>> 
>> {};
>> ---
>> 
>> Oh well, at any rate, you shouldn't need to refer to the type like this anymore:
>> ExtendOpTraits<SCEVSignExtendExpr>::GetExtendExprTy
>> 
>> 
>> REPOSITORY
>>  rL LLVM
>> 
>> http://reviews.llvm.org/D7645
>> 
>> EMAIL PREFERENCES
>>  http://reviews.llvm.org/settings/panel/emailpreferences/
>> 
>> 





More information about the llvm-commits mailing list